| Type | Bug | Status | reviewed | Date | 7-Mar-2014 18:09 |
|---|---|---|---|---|---|
| Version | r3 master | Category | Parse | Submitted by | kealist |
| Platform | All | Severity | minor | Priority | normal |
| Summary | TO/THRU do not give an error when given a rule as a target |
|---|---|
| Description | As @earl suggested this should return an error. If TO/THRU will not accept rules as targets, then it should trigger an error for an invalid parse rule. |
| Example code |
>> rule: [copy x thru some "0" (print x)] == [copy x thru some "0" (print x)] >> parse "000000" rule == false |
| Assigned to | n/a | Fixed in | - | Last Update | 7-Mar-2014 18:54 |
|---|
| Comments | |
|---|---|
|
(0004346)
BrianH 7-Mar-2014 18:15 |
Agreed. Everything we don't support should trigger an error, or else we'll never be able do debug such things, nor will we be able to add the feature later. Silent failures are the worst. |
|
(0004347)
kealist 7-Mar-2014 18:20 |
Sorry, maybe my code is a little off here, this does return an error:
> rule: [copy x thru [some "0"] (print x)] == [copy x thru [some "0"] (print x)] >> >> parse "000000" rule ** Script error: PARSE - invalid rule or usage of rule: some ** Where: parse ** Near: parse "000000" rule So, the question here is, should my invalid syntax above be causing an error? |
|
(0004348)
BrianH 7-Mar-2014 18:36 |
The syntax of TO and THRU is currently restricted by design, for really significant performance reasons. Having them accept rules at all was an enhancement, and those rules are limited to what could still be done while not losing the performance. For that matter, the restriction has a benefit because by sticking to those restrictions and instead doing the workaround code that is equivalent to your code, you speed up your PARSE rules. Adding that feature makes it easier to write really slow PARSE rules.
Given that this is part of the design, the code you wrote was in error. The problem here is that this error is not triggered by PARSE. The reason that this is a problem is that every silent failure enables buggy code that still works somehow, which we won't want to break in the future because the "works somehow" is important. PARSE is not like regular Rebol, it's a restricted dialect for a particular use. We (currently?) can't have modular extensions to PARSE. So, every extension needs to not break a lot of useful code, particularly after 3.0. And the main way to make those kinds of extensions possible is to make sure that they don't conflict with existing code patterns, even the erroneous ones. So to make this kind of thing possible in the future, we need to make it an error now, to make sure that conflicting code never gets written. |
|
(0004349)
abolka 7-Mar-2014 18:50 |
It's actually worse than the title suggests:
Currently, for string parsing TO or THRU don't cause an error when passed _any_ unknown or invalid argument. They just silently fail. That makes debugging a mess. Another example: >> parse "foo" [to 1.2] == false ;; Expected: ** Script error: PARSE - invalud rule or usage of rule: to Not handling seemingly valid composite rule constructs such as the `thru some "0"` in the example code is currently only a special case of this more general error. |
|
(0004351)
abolka 7-Mar-2014 18:54 |
The underlying problem of not causing an error for TO or THRU arguments which are not supported (by design) is fixed in https://github.com/rebol/rebol/pull/204.
The design question of allowing more types of arguments for TO or THRU (such as the recently added bitset! support, see #1457) is probably discussed in separate tickets. I think we even have a ticket for general rule support for TO/THRU around already. |
| Date | User | Field | Action | Change |
|---|---|---|---|---|
| 7-Mar-2014 18:54 | abolka | Comment : 0004351 | Added | - |
| 7-Mar-2014 18:50 | abolka | Comment : 0004349 | Added | - |
| 7-Mar-2014 18:36 | BrianH | Comment : 0004348 | Added | - |
| 7-Mar-2014 18:20 | kealist | Comment : 0004347 | Modified | - |
| 7-Mar-2014 18:20 | kealist | Comment : 0004347 | Added | - |
| 7-Mar-2014 18:15 | BrianH | Comment : 0004346 | Added | - |
| 7-Mar-2014 18:13 | BrianH | Status | Modified | submitted => reviewed |
| 7-Mar-2014 18:13 | BrianH | Type | Modified | Issue => Bug |
| 7-Mar-2014 18:12 | kealist | Code | Modified | - |
| 7-Mar-2014 18:12 | kealist | Type | Modified | Bug => Issue |
| 7-Mar-2014 18:09 | kealist | Ticket | Added | - |