Skip to content

False macro expansion error in lazy_static_include_bytes #11521

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
Tracked by #11704
matklad opened this issue Feb 21, 2022 · 5 comments
Open
Tracked by #11704

False macro expansion error in lazy_static_include_bytes #11521

matklad opened this issue Feb 21, 2022 · 5 comments
Labels
A-macro macro expansion Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now

Comments

@matklad
Copy link
Member

matklad commented Feb 21, 2022

This macro call fails to expand for me:

https://github.com/near/nearcore/blob/d338e91390c9b3ed422c66dbfd2548bcc8282249/genesis-tools/genesis-csv-to-json/src/csv_parser.rs#L345-L347

I think this is related to $vis token producing an empty match, as the following works:

pub RES => "res/test_accounts.csv"

@matklad matklad added A-macro macro expansion Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug labels Feb 21, 2022
@Veykril Veykril self-assigned this Apr 7, 2022
@Veykril
Copy link
Member

Veykril commented Apr 7, 2022

smaller repro:

macro_rules! m {
    (($tt:tt)) => {
        stringify!($tt)
    };
    ($vis:vis $name:ident) => {
        m!(($vis))
    };
}
const _: &str = m$0!(foo);

Turns out tt fragments can capture empty vis captures

@matklad
Copy link
Member Author

matklad commented Apr 7, 2022

🙊

@Veykril
Copy link
Member

Veykril commented Apr 7, 2022

Ye I don't think there is much we can do here right now? I assume rust doesn't need to convert intermediate expansions back into proper ASTs like we do 😬 so this requires us to have an empty ast node or something similar.

I demand a cursed label for macro related issues like these

@Veykril Veykril added the S-unactionable Issue requires feedback, design decisions or is blocked on other work label Apr 7, 2022
@Veykril Veykril removed their assignment Apr 7, 2022
@Veykril Veykril removed the Broken Window Bugs / technical debt to be addressed immediately label Apr 7, 2022
@Veykril
Copy link
Member

Veykril commented Apr 7, 2022

Oh now thinking about that, it makes a lot of sense that it works this way, since macro captures that arent tt, ident or lifetime are opaque, so capturing an empty token tree should be opaque to following macros 🙃. Ye I am really not sure how we can get out of this aside from modifying our syntax trees... lovely.

@matklad
Copy link
Member Author

matklad commented Apr 7, 2022

I think modifying syntax trees is ok: we support $crate in the ast for example.

One worry I have is that we lightly enforce an invariat that there are no nodes of length zero, and, if we allow them, I expect a tonne of subtle bugs to surface over the years.

So perhaps we can say that, in the ast, this node contains a single space?

@Veykril Veykril added E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now S-unactionable Issue requires feedback, design decisions or is blocked on other work Broken Window Bugs / technical debt to be addressed immediately and removed S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Apr 7, 2022
@Veykril Veykril removed the S-unactionable Issue requires feedback, design decisions or is blocked on other work label Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion Broken Window Bugs / technical debt to be addressed immediately C-bug Category: bug E-unknown It's unclear if the issue is E-hard or E-easy without digging in S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

No branches or pull requests

2 participants