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

Rewriter needs to account for "dead code" #2938

Closed
retailcoder opened this issue Mar 30, 2017 · 6 comments
Closed

Rewriter needs to account for "dead code" #2938

retailcoder opened this issue Mar 30, 2017 · 6 comments
Assignees
Labels
antlr Issue is easier to resolve with knowledge of Antlr4 critical Marks a bug as a must-fix, showstopper issue discussion enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope.

Comments

@retailcoder
Copy link
Member

As of recently, all module-rewriting operations are carried through a TokenStreamRewriter. That's a huge leap forward and greatly improves undo-ability and overall module-rewriting accuracy, however there's a problem that wasn't addressed:

TokenStreamRewriter works off the lexer token stream, and that stream is "cleansed" so that the parser can work around a number of limitations:

  • Line numbers are seen as WS tokens
  • Precompiler blocks that aren't "live" are seen as WS tokens

Therefore, piping all module changes to TSR's has also introduced a nasty side-effect: we're turning the user's modules into how Rubberduck sees them. And that doesn't include line numbers and "dead" precompiler code paths.

We can't have all precompiler code paths in the token streams, and despite all our efforts anything that tries to get line numbers picked up by a parser rule breaks the grammar.

We can't release anything until we've found a solution to this problem - how do we go about it?

@retailcoder retailcoder added antlr Issue is easier to resolve with knowledge of Antlr4 critical Marks a bug as a must-fix, showstopper issue discussion enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope. labels Mar 30, 2017
@retailcoder
Copy link
Member Author

One possibility would be, as soon as we have the token stream for a module (well, after it's given to the parser for processing, I suppose), to get its rewriter and immediately replace the WS tokens we introduced with the line numbers and dead preprocessor code paths that we stripped from the module content string.

@Hosch250
Copy link
Member

I'll take a crack at getting line numbers in the grammar. Can you provide different use cases I would need to account for?

@MDoerner
Copy link
Contributor

Might we be able to solve the problem with the preprocessing directives by sending the parts to ignore to another channel instead of replacing them? Obviously, that would need some external variables in the lexer.

@retailcoder
Copy link
Member Author

@MDoerner that's absolutely worth considering! ...except, wouldn't it mean we need embed the preprocessing logic into the lexer itself?

@comintern
Copy link
Contributor

@Hosch250 - When I was looking at adding line numbers and labels to the grammar, I determined that it shouldn't be horribly difficult but it will probably require a bit of work. I think the key is to add something like an endOfLine token - basically any newline that doesn't have a line continuation. The main problem is that ANTLR uses greedy matching, so and lineNumber\label token would need to either back-reference to an endOfLine or be explicitly endOfLine (lineNumber | lineLabel). Like I said, shouldn't be difficult - just requires a sh!tload of adjustments to other contexts to make it work.

Re preprocessor directives, would it work if we just sent that token stream to a different ANTLR channel?

@MDoerner
Copy link
Contributor

@retailcoder I think we can put them on another channel outside the lexer. The CommonToken has a settable field for the channel. So I think we can rewrite the channel during preprocessing.

MDoerner added a commit to MDoerner/Rubberduck that referenced this issue May 5, 2017
MDoerner added a commit to MDoerner/Rubberduck that referenced this issue May 5, 2017
@MDoerner MDoerner mentioned this issue May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
antlr Issue is easier to resolve with knowledge of Antlr4 critical Marks a bug as a must-fix, showstopper issue discussion enhancement Feature requests, or enhancements to existing features. Ideas. Anything within the project's scope.
Projects
None yet
Development

No branches or pull requests

4 participants