Skip to content

$crate is improperly resolved if it passes through another macro #6788

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

Closed
jonas-schievink opened this issue Dec 9, 2020 · 3 comments
Closed
Labels
A-macro macro expansion C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-hard S-actionable Someone could pick this issue up and work on it right now

Comments

@jonas-schievink
Copy link
Contributor

Current behavior:

//- /main.rs crate:main deps:lib
use lib::path;

macro_rules! vec {
  ( $e:expr ) => { $e };
}

fn f() {
    path![];
  //^^^^^^^ could not resolve macro `$crate::inner`
}

//- /lib.rs crate:lib
#[macro_export]
macro_rules! path {
    () => { vec![$crate::inner!()] };
}
#[macro_export]
macro_rules! inner {
    () => { 0 };
}

Expected behavior: The macro should be resolved correctly.

This causes incorrect diagnostics in crates/hir_ty/src/diagnostics/expr.rs, and probably all places that invoke the path! macro.

@jonas-schievink jonas-schievink added the A-macro macro expansion label Dec 9, 2020
@jonas-schievink
Copy link
Contributor Author

I think $crate needs to be resolved relative to the crate that contains the $crate tokens. I don't think we can do that in the current architecture, since we don't track hygiene of individual identifiers at the moment.

@lnicola lnicola added S-actionable Someone could pick this issue up and work on it right now C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) labels Dec 20, 2020
bors bot added a commit that referenced this issue Jan 2, 2021
7133: Proper handling $crate and local_inner_macros r=jonas-schievink a=edwin0cheng

This PR introduces `HygineFrames` to store the macro definition/call site hierarchy in hyginee and when resolving `local_inner_macros` and `$crate`, we use the token to look up the corresponding frame and return the correct value.

See also: https://rustc-dev-guide.rust-lang.org/macro-expansion.html#hygiene-and-hierarchies

fixe #6890 and  #6788

r? @jonas-schievink 

Co-authored-by: Edwin Cheng <[email protected]>
@lnicola
Copy link
Member

lnicola commented Jan 2, 2021

Fixed in #7133.

@lnicola lnicola closed this as completed Jan 2, 2021
@lnicola lnicola reopened this Jan 3, 2021
@edwin0cheng
Copy link
Member

Fixed in #7145 again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion C-Architecture Big architectural things which we need to figure up-front (or suggestions for rewrites :0) ) E-hard S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

3 participants