RAP 17: improved support for working with ambigous trees#82
RAP 17: improved support for working with ambigous trees#82PieterOlivier wants to merge 4 commits intowebsite-v2from
Conversation
5232ce4 to
979497e
Compare
There was a problem hiding this comment.
Ok cool.
Am I right in assuming that the proposed changes only activate if the code currently throws a static error? That would make them backwards compatible in a sense. Maybe add a section on the topic of compatibility with old code.
Is there already a section on what the current checker thinks of this? I suspect it already behaves as proposed. This would make the interpreter buggy (it's a bug , not a feature 😇
|
I have added section on backwards compatibility as there is a slight chance old code is affected. As this is purely a runtime feature I think the typechecker is not really a factor. As discuessed, the compiler will have to be adapted at some point to implement the proposed semantics, hopefully this will not be a big deal. I would not be surprised if it would only require small changes to the compiler runtime but @PaulKlint can answer this for sure. |
| ### Use of different exceptions for faulty field access on error trees and ambiguous trees | ||
| Error recovery often causes ambiguities even when the syntax does not normally allow ambigous trees. Separation between true ambiguities and ambiguities caused by | ||
| error recovery can be done by checking the alternatives of an ambiguous tree for error subtrees. | ||
| In practice we expect most ambiguities to be caused by error recovery so throwing only `ParseErrorRecovery` exceptions seems to offer the easiest path for developers. |
There was a problem hiding this comment.
I disagree here, I think the exception should be as clear as it can be. The previous sentence explains that we could even distinguis between the cause for the ambuity. So I would prefer to only throw the ParseErrorRecovery if we're sure the amb cluster came from error recovery. But if it's a different ambiguity source then throw an exception.
We already have an Ambiguity exception, that gets thrown by the parse function if you don't allow ambiguities. @jurgenvinju do you think it's wise to reuse that one here?
| @@ -31,4 +31,5 @@ not be turned into actual maintenance projects on the language. Completed RAPs a | |||
| * [ ] ((RAP14)) - Backward compatibility for Rascal modules | |||
| * [ ] ((RAP15)) - Conditional patterns and removal of accidental non-linear matching | |||
| * [ ] ((RAP16)) - Support for error trees | |||
There was a problem hiding this comment.
we could add an [x] in from of support for error trees, as we implemented that. (and add which version of rascal was the first one with the support for this, I think 0.42.0)
Currently when invalid access is attempted on an ambiguous tree a static error is thrown. Static errors cannot be caught in Rascal, making it necessary to check for ambiguous trees before any access.
This RAP proposes better support for working with ambiguous trees by throwing the same dynamic exception that is also thrown when a non-existent child of an error tree is accessed.