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

Expanded code using edition 2018 instead of 2021 #47

Closed
Tracked by #31
Veetaha opened this issue Jun 26, 2022 · 10 comments
Closed
Tracked by #31

Expanded code using edition 2018 instead of 2021 #47

Veetaha opened this issue Jun 26, 2022 · 10 comments
Labels
D-accepted A decision (D) has been made and the issue will be worked on I-bug This issue (I) regards a (potential) bug in the project T-accepted Triage (T): Initial review accepted issue/PR as valid

Comments

@Veetaha
Copy link

Veetaha commented Jun 26, 2022

I've found out that because this crate uses rust edition 2018, the code that we write inside of duplicate! {} macro is also forced to use edition 2018.

For example the following code panics with the message {a} instead of using the format args interpolation that was added in edition 2021:

fn main() {
    duplicate::duplicate! {
        [foo; [];]

        let a = 42;

        panic!("{a}");
    }
}

Also, the compiler warns about this when compiling:

warning: panic message contains an unused formatting placeholder
 --> crates/scratch/src/main.rs:7:17
  |
7 |         panic!("{a}");
  |                 ^^^
  |
  = note: `#[warn(non_fmt_panics)]` on by default
  = note: this message is not used as a format string when given without arguments, but will be in Rust 2021
help: add the missing argument
  |
7 |         panic!("{a}", ...);
  |                     +++++
help: or add a "{}" format string to use the message literally
  |
7 |         panic!("{}", "{a}");
  |                +++++

I've stumbled with this behavior and had to think really hard to deduce where the 2018 edition comes from because my crate where I use this macro is of edition 2021 🤨.

Anyway, I suppose this is a good reason to migration to the new edition.

@Veetaha Veetaha added D-discussion A decision (D) has not been made yet and is open to discussion I-bug This issue (I) regards a (potential) bug in the project labels Jun 26, 2022
@github-actions github-actions bot added the T-new Triage (T): Has yet to be reviewed label Jun 26, 2022
@Emoun Emoun added T-accepted Triage (T): Initial review accepted issue/PR as valid and removed T-new Triage (T): Has yet to be reviewed labels Jun 27, 2022
@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

@Veetaha thank you for this bug report.

It seems unlikely to me that the edition of duplicate should have any effect on the code as duplicate doesn't perform any execution itself, it just produces code. I would find it weird that rustc would enforce different editions for expanded code than non-expanded code.

I will investigate this and come back

@Emoun Emoun changed the title Migrate the crate to edition = "2021" Duplicated code using edition 2018 instead of 2021 Jun 27, 2022
@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

I have reproduced the given example and can verify that indeed duplicate's edition is enforced on the code.

Investigating this, macros apparently have 'edition hygiene' which cause this.

@Emoun Emoun mentioned this issue Jun 27, 2022
12 tasks
@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

Changing duplicate's edition to 2021 would be a breaking change. Both because it may cause previously working use-cases from breaking since the edition of the produced code changes, and because the MSRV is 1.42, which is earlier than 1.56 which is the first edition 2021 rustc version.

Also, simply changing duplicate's edition does not solve the problem in the long run. When the next edition is released, this problem returns.
I will have to investigate if there is a more permanent solution such that the expanded code always uses the edition of the caller instead of the edition of duplicate.

@Emoun Emoun changed the title Duplicated code using edition 2018 instead of 2021 Expanded code using edition 2018 instead of 2021 Jun 27, 2022
@Veetaha
Copy link
Author

Veetaha commented Jun 27, 2022

This limitation applies to any macro crate then 🤔 . That's an interesting semver hazard. I don't think there is a permanent solution to this problem. I also wonder what happens when a 2018 crate uses a 2021 macro.

@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

There does seem to be a possible solution:

I created a dummy macro that just expands to the code it is given:

#[proc_macro]
pub fn test_macro(stream: TokenStream) -> TokenStream
{
	stream
}

Calling this macro instead of duplicate with the above example does result in edition 2021 being used, even if the macro itself uses edition 2018. I suspect this is because the token stream is marked as being 2021 code, and since we just return it, this is maintained.
duplicate on the other hand does manipulate the code it gets to perform substitutions and such, so I can imagine that somewhere we create new tokens which are then automatically marked with duplicate's edition.

So, I think it might be possible to take this into account and make a permanent solution. Question now becomes how to test it.

@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

That's an interesting semver hazard

Maintaining this crate has really shown me how convoluted and pervasive semver compatibility is. Hazards everywhere.

@Emoun
Copy link
Owner

Emoun commented Jun 27, 2022

Edition hygiene seems to be tracked by spans. The following test macro changes all token spans to be Span::call_site:

#[proc_macro]
pub fn test_macro(stream: TokenStream) -> TokenStream
{
	TokenStream::from_iter(stream.into_iter().map(|mut t| {
		t.set_span(Span::call_site());
		t
	}))
}

Using it on the example results in the edition of the macro always being used.

@Emoun Emoun added D-accepted A decision (D) has been made and the issue will be worked on and removed D-discussion A decision (D) has not been made yet and is open to discussion labels Jun 28, 2022
@Emoun
Copy link
Owner

Emoun commented Jul 8, 2022

Fix implemented and awaiting next release.

@Veetaha could you try out the latest commit on master to see if it fixes all your problems? I have a test for your specific use case above but have not found any other cases where this happens.

@Veetaha
Copy link
Author

Veetaha commented Jul 13, 2022

@Emoun indeed, the patch does fix my problem. I don't have any other use cases though, so let's consider this resolved? Also, thanks for accurately investigating this 😋!

@Emoun
Copy link
Owner

Emoun commented Jul 13, 2022

@Veetaha you're welcome and thanks for testing it for me. I'll see about releasing a new version at some point this week(end).

@Emoun Emoun closed this as completed Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D-accepted A decision (D) has been made and the issue will be worked on I-bug This issue (I) regards a (potential) bug in the project T-accepted Triage (T): Initial review accepted issue/PR as valid
Projects
None yet
Development

No branches or pull requests

2 participants