| Type | Wish | Status | problem | Date | 9-Apr-2013 12:35 |
|---|---|---|---|---|---|
| Version | r3 master | Category | Parse | Submitted by | abolka |
| Platform | All | Severity | minor | Priority | normal |
| Summary | Allow set-word! for the word argument of PARSE's SET and COPY rules |
|---|---|
| Description |
(The following discussion will, for brevity, only refer to SET. COPY is to be treated fully analogous to SET.) Currently, PARSE's SET keyword takes a "word" argument, which must be a word! type: >> parse [42] [set foo integer!] foo == 42 Using a set-word! as argument, casues an error: >> parse [42] [set foo: integer!] foo ** Script error: PARSE - expected a variable, not: foo: I suggest to allow set-word! types as argument to PARSE's SET as well. The main benefit would be that this usage allows us to indicate to e.g. FUNCT that these variables ought to be "local". Currently, PARSE variables "leak" through e.g. FUNCT, requiring us to explicitly handle PARSE variables that ought to be local: f: funct [s /local bar] [parse s [set bar integer!] bar] Allowing set-word! would allow us to forego this explicit listing: g: funct [s] [parse s [set baz: integer!] baz] In this case, FUNCT (using COLLECT-WORDS/set/deep) would be able to detect and properly handle BAZ as local. The operation of PARSE's SET would not change, and the current behaviour of SET word! would still stay in place. So this is a pure extension of functionality, which should have no effect on backwards compatibility, except for previously erroneous code now becoming meaningful. |
| Example code |
| Assigned to | n/a | Fixed in | r3 master | Last Update | 7-Mar-2014 20:17 |
|---|
| Comments | |
|---|---|
|
(0003809)
adrians 9-Apr-2013 18:16 |
This would be great. Is backwards compatibility the only reason for allowing the current SET word behaviour to remain? |
|
(0003810)
abolka 9-Apr-2013 19:36 |
No, that's not the only reason. (But backwards compatibility certainly is one reason as well.)
Must fundamentally, there simply is no good reason for removing or changing `SET word` behaviour. `SET word` is independently still very useful: when variable localisation is not necessary and you consider `SET word` more readable, or when you explicitly want to avoid automatic locals handling, and set a "global" variable. Basically, it's the same as with `SET 'name` vs `name:` in the DO dialect. Both are useful. |
|
(0003811)
BrianH 9-Apr-2013 20:38 |
This wouldn't change the behavior, this would just allow the variable used by the current SET and COPY operations to be a set-word, in addition to its existing support for that variable being a word. There is no existing interpretation for a set-word in that position that would be lost here, so from a semantic standpoint nothing would be lost, and this syntax could be understood by the dialect processor without confusion.
The main downside is that this will make the PARSE dialect harder to understand by developers. Even though there would be no conflict to the dialect processor between set-words used as variables for SET and COPY and set-word operations (position getting, just like in *EACH), it would make the PARSE dialect more confusing to people reading parse rules. For instance: >> parse [5] [set x: y: integer!] reduce [x y] == [5 [5]] At a glance it is hard to tell that x: is doing one thing and y: is doing something else. So, this would be a tradeoff, making it easier to declare local variables, at the expense of making it harder to debug your code. Maybe this is a tradeoff worth making, and it well may be, since people who use the locals-gathering function builders are more likely to be careful with set-words in their code already. |
|
(0003816)
Ladislav 10-Apr-2013 11:27 |
Indeed, this seems as a reasonable tradeoff to make. |
|
(0004012)
fork 25-Sep-2013 15:06 |
Was going to add this ticket, but I see it already exists, and there is no opposition....so...
Can we go ahead and make the change? It looks like as the parser moves along the rule, if it sees a COPY or SET, then it sets the flag PF_COPY or PF_SET as appropriate. (Note: it actually sets *both* in the case of copy, so you need to test the flags in order and assume they're exclusive... eeek, confusing micro-optimization! :-/) https://github.com/rebol/rebol/blob/bf2ec8dd84abc2af50cc9bda5cc1548f0ce00ce8/src/core/u-parse.c#L707 Note that the specific "expected a variable" error you get when you currently say COPY X: THRU FOO comes from a test to make sure the next element is a WORD!. This would have to be changed to IS_WORD(item) || IS_SET_WORD(item), for starters. But merely enabling that would likely lead to it saving the parse position and ignoring the copy/set flags here: https://github.com/rebol/rebol/blob/bf2ec8dd84abc2af50cc9bda5cc1548f0ce00ce8/src/core/u-parse.c#L793 So off the cuff I'd guess that if SET-WORD! was passed in the initial test and the flags set, you could check the flags there. If they are set, you could make it do what WORD! does, setting item to the variable indicated by the set-word and falling through instead of doing a CONTINUE: https://github.com/rebol/rebol/blob/bf2ec8dd84abc2af50cc9bda5cc1548f0ce00ce8/src/core/u-parse.c#L812 Then as it proceeds, when it hits a WORD! it checks to see if the flags are set, and if so it will use that word as the named place to copy or set: https://github.com/rebol/rebol/blob/bf2ec8dd84abc2af50cc9bda5cc1548f0ce00ce8/src/core/u-parse.c#L988 That copying and setting code is inside of an IF IS_WORD, thus it would not allow SET-WORD! at the moment: https://github.com/rebol/rebol/blob/bf2ec8dd84abc2af50cc9bda5cc1548f0ce00ce8/src/core/u-parse.c#L864 It may be easiest in the code where the flags are checked and the set-word is recognized to just switch it to be a word, or add IS_SET_WORD to the condition and hammer out the consequences. |
|
(0004033)
fork 3-Oct-2013 20:15 |
Implementation seems simpler than I thought. Believed to be accomplished with https://github.com/rebol/rebol/pull/165 |
|
(0004353)
BrianH 7-Mar-2014 20:17 |
There is a bug in the current implementation. See #2130 for details. |
| Date | User | Field | Action | Change |
|---|---|---|---|---|
| 7-Mar-2014 20:17 | BrianH | Comment : 0004353 | Added | - |
| 7-Mar-2014 20:15 | BrianH | Status | Modified | built => problem |
| 17-Feb-2014 22:20 | abolka | Fixedin | Modified | => r3 master |
| 17-Feb-2014 22:20 | abolka | Status | Modified | reviewed => built |
| 5-Jan-2014 02:31 | abolka | Description | Modified | - |
| 5-Jan-2014 02:31 | abolka | Status | Modified | submitted => reviewed |
| 3-Oct-2013 20:16 | fork | Comment : 0004033 | Modified | - |
| 3-Oct-2013 20:15 | fork | Comment : 0004033 | Added | - |
| 25-Sep-2013 15:06 | fork | Comment : 0004012 | Added | - |
| 10-Apr-2013 11:27 | Ladislav | Comment : 0003816 | Added | - |
| 9-Apr-2013 21:20 | BrianH | Comment : 0003811 | Modified | - |
| 9-Apr-2013 20:38 | BrianH | Comment : 0003811 | Added | - |
| 9-Apr-2013 19:36 | abolka | Comment : 0003810 | Added | - |
| 9-Apr-2013 18:16 | adrians | Comment : 0003809 | Added | - |
| 9-Apr-2013 12:35 | abolka | Ticket | Added | - |