Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More precise code-completion for ANTLRv4 Grammars #4808

Merged
merged 4 commits into from
Oct 25, 2022

Conversation

lkishalmi
Copy link
Contributor

Well, just a quick one. Lexer grammars, can only reference fragments inside rules, pure parser grammars cannot reference fragments. Add some heuristic to support lexercommands as well. + icons

@lkishalmi lkishalmi added this to the NB16 milestone Oct 17, 2022
@neilcsmith-net neilcsmith-net modified the milestones: NB16, NB17 Oct 19, 2022
@neilcsmith-net
Copy link
Member

Sorry, hadn't a chance to review (and no-one else had) prior to branching. Please make a call and rebase onto delivery if you think this should go into 16-rc2.

@lkishalmi lkishalmi changed the base branch from master to delivery October 19, 2022 14:10
for (String im : imports) {
FileObject ifo = getFileObject().getParent().getFileObject(im + ".g4");
if (ifo != null) {
Antlr4ParserResult pr = (Antlr4ParserResult) AntlrParser.getParserResult(ifo);
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this blow when grammar files form a cycle? Assuming we have a direct cycle A -> B, B -> A, then:

  • user ask for the parser result for file A
  • the parse process will hit this section and ask for the parser result for file B
  • the parser result is not found in the cache, so the parsing for B is invoked
  • parsing for B will hit this section and ask for the parser result of file A

The cycle is closed. I just noticed, that we access other files from the parser. I think this is an error. This kind of error checking needs to be done outside the parser, then you can access the parser results sequentially and build whatever structure you need.

Copy link
Contributor

Choose a reason for hiding this comment

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

This will also create stale errors - if you add a definition to import, there is no reason to invoke the Parser for the including file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I hope the new implementation is looking better.

@lkishalmi lkishalmi modified the milestones: NB17, NB16 Oct 24, 2022
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

If I understand this correctly this works safely because a hard reference to the Antlr4ParserResults is kept and thus we will not recurse indefinitely. Lets get this in.

@lkishalmi
Copy link
Contributor Author

Yes for the time the imports are processed we keep a hard reference on the results.

@neilcsmith-net neilcsmith-net merged commit c07fd58 into apache:delivery Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants