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

Use Span::mixed_site to avoid let unit value warnings #2586

Closed
wants to merge 1 commit into from

Conversation

jieyouxu
Copy link
Contributor

Using Span::mixed_site changes the syntax context of the generated responder outcome expr to identify as macro-expanded, helping the clippy::let_unit_value lint and a potential unit_bindings rustc lint to avoid linting on generated code.

Closes #2568.
Avoids a potential rustc unit_bindings lint (if it is merged at rust-lang/rust#112380).

@SergioBenitez
Copy link
Member

SergioBenitez commented Aug 26, 2023

Is this correct? Doesn't this change hygiene?

@jieyouxu
Copy link
Contributor Author

jieyouxu commented Aug 26, 2023

Is this correct? Doesn't this change hygiene?

It does indeed change hygiene AFAIUI, in that it becomes mixed hygienic. That is,

Procedural macro producing tokens with Span::mixed_site() spans should be equivalent to a macro_rules macro producing the same tokens.

My understanding (might be wrong here) is that originally the proc_macro was unhygienic w.r.t. local variables (which might be why ___responder has the three underscores?) and defaults to Span::call_site hygiene. Using Span::mixed_site().located_at(ret_span) would change the macro from being unhygienic w.r.t. local variables to being mixed hygienic w.r.t. local variables but retain the line/column information of ret_span.

(If my understanding is correct, there probably is a couple more places that probably should be Span::mixed_site instead of directly forwarding the user code's span which has Span::call_site hygiene by default, since Span::call_site hygiene would expose local bindings introduced within the proc-macros to user code I think?)

@SergioBenitez
Copy link
Member

I've merged this. Let's see how it goes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: merged This pull request was merged manually.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive clippy::let_unit_value in empty route
2 participants