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

feat: Fix up syntax errors in attribute macro inputs to make completion work more often #11444

Merged
merged 15 commits into from
Feb 12, 2022

Conversation

flodiebold
Copy link
Member

This implements the "fix up syntax nodes" workaround mentioned in #11014. It isn't much more than a proof of concept; I have only implemented a few cases, but it already helps quite a bit.

Some notes:

  • I'm not super happy about how much the fixup procedure needs to interact with the syntax node -> token tree conversion code (e.g. needing to share the token map). This could maybe be simplified with some refactoring of that code.
  • It would maybe be nice to have the fixup procedure reuse or share information with the parser, though I'm not really sure how much that would actually help.

@flodiebold
Copy link
Member Author

Proof that it works:
Peek 2022-02-09 20-32

@Veykril
Copy link
Member

Veykril commented Feb 11, 2022

I'm not super happy about how much the fixup procedure needs to interact with the syntax node -> token tree conversion code (e.g. needing to share the token map). This could maybe be simplified with some refactoring of that code.

Ye I agree, this interaction seems rather difficult to get behind. I hope there is some cleanup we can do here.

@flodiebold
Copy link
Member Author

@Veykril any objections to merging it in the current state though?

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

No objections here,

one thing to note as the fixup stuff is only really relevant for completions, I am not too fond of the fixup stuff being generated for all other macro expansion query calls that aren't interested in it(though this probably is a non-issue)
Forget this comment we do need it for everything probably(due to diagnostics etc)

@flodiebold
Copy link
Member Author

We do need the fixup for the expansion during normal analysis even for completions anyway, since we (currently) can't do things like name resolution inside the speculative expansion.

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 12, 2022

@bors bors bot merged commit 7a17fb9 into rust-lang:master Feb 12, 2022
@flodiebold flodiebold deleted the proc-macro-syntax-completion branch February 12, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants