Skip to content

Commit

Permalink
Make analyzer in Pico's language server resilient to parse errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@sungshik
sungshik committed Nov 11, 2024
1 parent 383a83c commit 5a36693
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion rascal-lsp/src/main/rascal/demo/lang/pico/LanguageServer.rsc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Summary picoSummaryService(loc l, start[Program] input, PicoSummarizerMode mode)
Summary s = summary(l);
// definitions of variables
rel[str, loc] defs = {<"<var.id>", var.src> | /IdType var := input};
rel[str, loc] defs = {<"<var.id>", var.src> | /IdType var := input, !hasErrors(var)};

// uses of identifiers
rel[loc, str] uses = {<id.src, "<id>"> | /Id id := input};
Expand Down

8 comments on commit 5a36693

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not what I had in mind for the error trees. They should work if they match a pattern up to meaningful interactions like field projection, without having to check for the error case.

See the proposal on the Rascal project for an extension of field projection on error trees. I believe that may be all there is to it. If field projection is robust then this code should work without checks.

Another possible avenue is to deal with the exceptions that come from error trees differently. One could have the match fail automaticsu for example if an error tree interaction throws a runtime error. That way the backtracking behavior of visit and match can deal with error cases automatically. Maybe both are needed?

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is very good to see how it could work right now, by the way. Great to do this example! I'm just sharing my own expectations and long-waiting ideas here. Groetjes!

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A field projection stands in place of a pattern match, or a disjunction of matches for all constructors that have that field. If it fails it could also trigger backtracking like pattern matches do. Or we have to find a way to write the disjunctivr field projection as a pattern match.

@sungshik
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, yes, thank you for these comments 🙂. There's indeed a more fundamental point/tension between: (a) code being statically well-typed; (b) getting a runtime error for an undefined field when a tree has an error node. Your proposal to extend field projection on error trees avoids the runtime error (if I understand correctly), but in this Pico example, I'm not sure if it gives meaningful results.

I wonder if we should have two kinds of visits: safe (default; used in := and <-) and unsafe (e.g., ?:= and ?<-). Safe visits would automatically skip any match that contains an error tree (or that throws an exception?), so they're statically type-sound; unsafe visits would not. I think this is maybe similar to what you suggest here:

One could have the match fail automaticsu for example if an error tree interaction throws a runtime error. That way the backtracking behavior of visit and match can deal with error cases automatically.

Anyway, @PieterOlivier is going to summarize the discussion so far in a central place somewhere and further explore the solution space.

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. @PieterOlivier @PaulKlint I think it's time for an old fashioned feature matrix. All features on normal parse trees times the error trees with their parameters (unfinished production rule, dot, skipped characters). An almost full matrix would mean almost fully robust client code.

@jurgenvinju
Copy link
Member

@jurgenvinju jurgenvinju commented on 5a36693 Nov 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see more possibilities to keep the error handling in a separate case and not tangled with the correct cases.

If the fields were projected during the case pattern already, for example, then this would fail the match and keep this case clean.

Or, if in a case block failed field projections would lead to the next case match like a fail statement; that would have the same effect of keeping the clean streats clean.

Maybe there are more ways to keep error handing a separate aspect. Since cases in visits are tried from top to bottom we can start all of them with a case that catches erroneous trees. For this we'd need the error counter on all nodes on the spine from root to error.

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need solutions that help in pattern dispatched functions.

@jurgenvinju
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I'm thinking maybe we should move the field projections to the matching side, such that they can fail with a well-defined alternative.

Please sign in to comment.