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

cargo fix doesn't work with projects using rocket (it wants to fix plugin-generated code) #6124

Closed
jplatte opened this issue Oct 3, 2018 · 8 comments

Comments

@jplatte
Copy link
Contributor

jplatte commented Oct 3, 2018

Steps to reproduce:

git clone https://github.com/jplatte/turbo.fish
cd turbo.fish
cargo fix --edition-idioms
@alexcrichton
Copy link
Member

cc @zackmdavis perhaps you could help out here as well?

I believe rust-lang/rust#52069 started adding the lints that are firing here which look like:

warning: hidden lifetime parameters in types are deprecated
  --> src/main.rs:33:1
   |
33 | #[get("/<turbofish>")]
   | ^^^^^^^^^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`

We should have safeguards in place for the compiler to not actually emit any warnings here, but somehow this lint isn't triggering these safeguards which means the suggestion isn't getting classified as "coming from an external macro", which means that we're not correctly suppressing the suggestion (I think the lint still needs to be emitted though).

@zackmdavis do you know why this might be getting past that check? Is perhaps the wrong span being used for the suggestion, one which doesn't tie it back to the original macro?

@zackmdavis
Copy link
Member

(I'll take a look sometime this week.)

@zackmdavis
Copy link
Member

@alexcrichton

I added some logging to a stage 2 build to get some more visibility

diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs
index 3327b11..c9146f6 100644
--- a/src/librustc/lint/mod.rs
+++ b/src/librustc/lint/mod.rs
@@ -619,7 +619,9 @@ pub fn struct_lint_level<'a>(sess: &'a Session,
     // If this code originates in a foreign macro, aka something that this crate
     // did not itself author, then it's likely that there's nothing this crate
     // can do about it. We probably want to skip the lint entirely.
-    if err.span.primary_spans().iter().any(|s| in_external_macro(sess, *s)) {
+    if err.span.primary_spans().iter()
+        .inspect(|&s| warn!("span {:?}; in_external_macro {:?}", s, in_external_macro(sess, *s)))
+        .any(|s| in_external_macro(sess, *s)) {

and the result—

 INFO 2018-10-19T05:10:10Z: rustc::lint: span Span { lo: BytePos(459), hi: BytePos(476), ctxt: #0 }; in_external_macro false
warning: hidden lifetime parameters in types are deprecated                                                           
  --> src/main.rs:28:1                                                                                                
   |                                                                                                                  
28 | #[get("/random")]                                                                                                
   | ^^^^^^^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`

(here #[get("/random")] indeed starts 459 bytes into the file, in accordance with the log line)

—makes me think that the span of the macro attribute itself (which, correctly, doesn't count as coming from a macro) is being passed as the path_span argument to lower_path_segment during HIR lowering (which argument is where the lint gets its primary span).

I don't currently know how we would fix this because I don't currently know much about procedural macros.

@alexcrichton
Copy link
Member

Thanks for taking a look at this @zackmdavis! That makes sense that it's the issue, although I'm also unsure why that would come about to have a span that looks like that. Procedural macros in theory don't have much to do with it other than they're creating pre-HIR ast nodes with "unusual spans", but that's always been enough in the past...

@ehuss
Copy link
Contributor

ehuss commented Nov 30, 2018

@alexcrichton I think the issue here is that the original code is using a plugin to register an attribute. Newer versions of Rocket use proc-macros, which seem to handle the edition lints correctly. I suspect handling plugin-generated code isn't worth the effort (or maybe it's the plugin's responsibility), maybe this should be closed wontfix?

@jplatte
Copy link
Contributor Author

jplatte commented Nov 30, 2018

@ehuss This happens with the latest release of Rocket, v3.1.7. Are you referring to unreleased versions?

@ehuss
Copy link
Contributor

ehuss commented Nov 30, 2018

Sorry, that may have been a premature suggestion. I was just assuming that due to the way that plugins are registered, it would be difficult to get the correct edition hygiene. But I do not fully understand how that works, so I probably shouldn't have suggested that.

Yes, I am referring to 0.4.0-rc.1.

@alexcrichton
Copy link
Member

@ehuss sounds reasonable to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants