Skip to content

Fix: mark relevant spans (ex: ident.span) with macro expansion context #66095

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

Open
Quantumplation opened this issue Nov 4, 2019 · 1 comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Quantumplation
Copy link
Contributor

In #65830 it was discovered that hir::Item.ident.span isn't marked with_ctxt of being part of a macro expansion. This had two effects that we saw there:

  • error messages don't include the "in this macro invocation" hints
  • cases where the dead_code error was suppressed because of foreign macros were no longer suppressed

To solve that there, we're going to just use the old behavior when we're in the middle of a macro expansion already, so we don't use the un-marked span. As @estebank suggested, we should probably clean this up in a separate change.

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Nov 5, 2019
Quantumplation added a commit to Quantumplation/rust that referenced this issue Nov 5, 2019
If item.span is part of a macro invocation, this has several downstream
implications.  To name two that were found while working on this:

 - The dead-code error gets annotated with a "in this macro invocation"
 - Some errors get canceled if they refer to remote crates

Ideally, we should annotate item.ident.span with the same macro info,
but this is a larger change (see: rust-lang#66095), so for now we just fall
back to the old behavior if this item was generated by a macro.

I use span.macro_backtrace().len() to detect if it's part of a macro,
because that (among other things) is what is used by the code which
adds the "in this macro invocation" annotations mentioned above.
@Quantumplation
Copy link
Contributor Author

@estebank when you get a chance could you (or someone else) provide some light mentorship here?
Specifically,

  • Where does item.span get constructed (so I can look at how the macro context gets attached)
  • Where does item.ident.span get filled in?
  • What other relevant spans should inherit this context?
  • Are there any gotchas I should consider?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants