Remove `#[cfg]` attributes during cfg-expansion by Aaron1011 · Pull Request #84110 · rust-lang/rust

Aaron1011 · 2021-04-11T22:09:21Z

Currently, we don't remove #[cfg] attributes from a target when the
predicates pass. This PR removes all 'passing' #[cfg] attributes
during cfg-expansion, which causes derive proc-macros to never see any
#[cfg] attributes in the input token stream.

With #82608 merged, we can now do
this without losing spans.

Currently, we don't remove `#[cfg]` attributes from a target when the predicates pass. This PR removes all 'passing' `#[cfg]` attributes during cfg-expansion, which causes derive proc-macros to never see any `#[cfg]` attributes in the input token stream. With rust-lang#82608 merged, we can now do this without losing spans.

rust-highfive · 2021-04-11T22:09:24Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2021-04-11T22:09:38Z

r? @petrochenkov

petrochenkov · 2021-04-13T21:16:37Z

@bors r+ rollup- (not evaluating cfg predicates repeatedly may be a performance improvement)

bors · 2021-04-13T21:16:39Z

📌 Commit b17a82d has been approved by petrochenkov

petrochenkov · 2021-04-13T21:17:54Z

@bors rollup=never

bors · 2021-04-13T21:43:26Z

⌛ Testing commit b17a82d with merge aa99cb5670ac26f1c282fe15f225d30da351a9fa...

rust-log-analyzer · 2021-04-13T22:24:23Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)


---- compile_test stdout ----
diff of stderr:

 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if y == "world" {
 LL | |             println!("world!")
 LL | |         }
 LL | |     }
    |
    |
    = note: `-D clippy::collapsible-else-if` implied by `-D warnings`
 help: collapse nested if block
    |
 LL |     } else if y == "world" {
 LL |         println!("world!")
    |
 
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if let Some(42) = Some(42) {
 LL | |             println!("world!")
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     } else if let Some(42) = Some(42) {
 LL |         println!("world!")
    |
 
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if y == "world" {
 LL | |             println!("world")
 LL | |         }
 LL | |         }
 LL | |     }
    | |_____^
    |
    |
 help: collapse nested if block
    |
 LL |     } else if y == "world" {
 LL |         println!("world")
 LL |     else {
 LL |         println!("!")
 LL |     }
    |
    |
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if let Some(42) = Some(42) {
 LL | |             println!("world")
 LL | |         }
 LL | |         }
 LL | |     }
    | |_____^
    |
    |
 help: collapse nested if block
    |
 LL |     } else if let Some(42) = Some(42) {
 LL |         println!("world")
 LL |     else {
 LL |         println!("!")
 LL |     }
    |
    |
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if let Some(42) = Some(42) {
 LL | |             println!("world")
 LL | |         }
 LL | |         }
 LL | |     }
    | |_____^
    |
    |
 help: collapse nested if block
    |
 LL |     } else if let Some(42) = Some(42) {
 LL |         println!("world")
 LL |     else {
 LL |         println!("!")
 LL |     }
    |
    |
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if x == "hello" {
 LL | |             println!("world")
 LL | |         }
 LL | |         }
 LL | |     }
    | |_____^
    |
    |
 help: collapse nested if block
    |
 LL |     } else if x == "hello" {
 LL |         println!("world")
 LL |     else {
 LL |         println!("!")
 LL |     }
    |
    |
 
 error: this `else { if .. }` block can be collapsed
    |
 LL |       } else {
    |  ____________^
    |  ____________^
 LL | |         if let Some(42) = Some(42) {
 LL | |             println!("world")
 LL | |         }
 LL | |         }
 LL | |     }
    | |_____^
    |
    |
 help: collapse nested if block
    |
 LL |     } else if let Some(42) = Some(42) {
 LL |         println!("world")
 LL |     else {
 LL |         println!("!")
 LL |     }
    |
    |
 
-error: aborting due to 7 previous errors
+error: this `else { if .. }` block can be collapsed
+   |
+LL |       } else {
+   |  ____________^
+   |  ____________^
+LL | |         #[cfg(not(roflol))]
+LL | |         if y == "world" {
+LL | |             println!("world!")
+LL | |         }
+LL | |     }
+   |
+   |
+help: collapse nested if block
+   |
+LL |     } else if y == "world" {
+LL |         println!("world!")
+LL |     }
+
+error: aborting due to 8 previous errors
 
 
 

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base/collapsible_else_if.stage-id.stderr

 // run-rustfix
 // run-rustfix
 #![allow(clippy::assertions_on_constants)]
 
 #[rustfmt::skip]
 #[warn(clippy::collapsible_if)]
 #[warn(clippy::collapsible_else_if)]
 fn main() {
     let x = "hello";
     let x = "hello";
     let y = "world";
     // Collapse `else { if .. }` to `else if ..`
     if x == "hello" {
         print!("Hello ");
     } else if y == "world" {
         println!("world!")
 
 
     if x == "hello" {
         print!("Hello ");
     } else if let Some(42) = Some(42) {
         println!("world!")
 
 
     if x == "hello" {
         print!("Hello ");
     } else if y == "world" {
         println!("world")
     else {
         println!("!")
     }
 
 
     if x == "hello" {
         print!("Hello ");
     } else if let Some(42) = Some(42) {
         println!("world")
     else {
         println!("!")
     }
 
 
     if let Some(42) = Some(42) {
         print!("Hello ");
     } else if let Some(42) = Some(42) {
         println!("world")
     else {
         println!("!")
     }
 
 
     if let Some(42) = Some(42) {
         print!("Hello ");
     } else if x == "hello" {
         println!("world")
     else {
         println!("!")
     }
 
 
     if let Some(42) = Some(42) {
         print!("Hello ");
     } else if let Some(42) = Some(42) {
         println!("world")
     else {
         println!("!")
     }
 
 
     if x == "hello" {
         print!("Hello ");
-    } else {
-        #[cfg(not(roflol))]
-        if y == "world" {
-            println!("world!")
-        }
+    } else if y == "world" {
+        println!("world!")
 }
 

The actual fixed differed from the expected fixed.
The actual fixed differed from the expected fixed.
Actual fixed saved to /checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base/collapsible_else_if.stage-id.fixed
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args collapsible_else_if.rs`
error: 2 errors occurred comparing output.
status: exit status: 1
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools-bin/clippy-driver" "tests/ui/collapsible_else_if.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base/collapsible_else_if.stage-id" "-A" "unused" "--emit=metadata" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/release/deps" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps" "-Dwarnings" "-Zui-testing" "--extern" "clippy_lints=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libclippy_lints-934c285f6858724f.rlib" "--extern" "quote=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libquote-7f4531ca9e916653.rlib" "--extern" "serde=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libserde-3365d689274e8da9.rlib" "--extern" "regex=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libregex-3f3ead7dae58a5a8.rlib" "--extern" "syn=/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/libsyn-43d16fd8e2fbc291.rlib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base/collapsible_else_if.stage-id.aux"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":308,"byte_end":382,"line_start":14,"line_end":18,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"`-D clippy::collapsible-else-if` implied by `-D warnings`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":308,"byte_end":382,"line_start":14,"line_end":18,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if y == \"world\" {\n        println!(\"world!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:14:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if y == \"world\" {\nLL | |             println!(\"world!\")\nLL | |         }\nLL | |     }\n   | |_____^\n   |\n   = note: `-D clippy::collapsible-else-if` implied by `-D warnings`\nhelp: collapse nested if block\n   |\nLL |     } else if y == \"world\" {\nLL |         println!(\"world!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":443,"byte_end":528,"line_start":22,"line_end":26,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":443,"byte_end":528,"line_start":22,"line_end":26,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if let Some(42) = Some(42) {\n        println!(\"world!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:22:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if let Some(42) = Some(42) {\nLL | |             println!(\"world!\")\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if let Some(42) = Some(42) {\nLL |         println!(\"world!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":589,"byte_end":713,"line_start":30,"line_end":37,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":589,"byte_end":713,"line_start":30,"line_end":37,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if y == \"world\" {\n        println!(\"world\")\n    }\n    else {\n        println!(\"!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:30:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if y == \"world\" {\nLL | |             println!(\"world\")\nLL | |         }\n...  |\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if y == \"world\" {\nLL |         println!(\"world\")\nLL |     }\nLL |     else {\nLL |         println!(\"!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":774,"byte_end":909,"line_start":41,"line_end":48,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":774,"byte_end":909,"line_start":41,"line_end":48,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if let Some(42) = Some(42) {\n        println!(\"world\")\n    }\n    else {\n        println!(\"!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:41:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if let Some(42) = Some(42) {\nLL | |             println!(\"world\")\nLL | |         }\n...  |\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if let Some(42) = Some(42) {\nLL |         println!(\"world\")\nLL |     }\nLL |     else {\nLL |         println!(\"!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":981,"byte_end":1116,"line_start":52,"line_end":59,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":981,"byte_end":1116,"line_start":52,"line_end":59,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if let Some(42) = Some(42) {\n        println!(\"world\")\n    }\n    else {\n        println!(\"!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:52:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if let Some(42) = Some(42) {\nLL | |             println!(\"world\")\nLL | |         }\n...  |\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if let Some(42) = Some(42) {\nLL |         println!(\"world\")\nLL |     }\nLL |     else {\nLL |         println!(\"!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1188,"byte_end":1312,"line_start":63,"line_end":70,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if x == \"hello\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1188,"byte_end":1312,"line_start":63,"line_end":70,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if x == \"hello\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if x == \"hello\" {\n        println!(\"world\")\n    }\n    else {\n        println!(\"!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:63:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if x == \"hello\" {\nLL | |             println!(\"world\")\nLL | |         }\n...  |\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if x == \"hello\" {\nLL |         println!(\"world\")\nLL |     }\nLL |     else {\nLL |         println!(\"!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1384,"byte_end":1519,"line_start":74,"line_end":81,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1384,"byte_end":1519,"line_start":74,"line_end":81,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        if let Some(42) = Some(42) {","highlight_start":1,"highlight_end":37},{"text":"            println!(\"world\")","highlight_start":1,"highlight_end":30},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"        else {","highlight_start":1,"highlight_end":15},{"text":"            println!(\"!\")","highlight_start":1,"highlight_end":26},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if let Some(42) = Some(42) {\n        println!(\"world\")\n    }\n    else {\n        println!(\"!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:74:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         if let Some(42) = Some(42) {\nLL | |             println!(\"world\")\nLL | |         }\n...  |\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if let Some(42) = Some(42) {\nLL |         println!(\"world\")\nLL |     }\nLL |     else {\nLL |         println!(\"!\")\nLL |     }\n   |\n\n"}
{"message":"this `else { if .. }` block can be collapsed","code":{"code":"clippy::collapsible_else_if","explanation":null},"level":"error","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1580,"byte_end":1682,"line_start":85,"line_end":90,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        #[cfg(not(roflol))]","highlight_start":1,"highlight_end":28},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"collapse nested if block","code":null,"level":"help","spans":[{"file_name":"tests/ui/collapsible_else_if.rs","byte_start":1580,"byte_end":1682,"line_start":85,"line_end":90,"column_start":12,"column_end":6,"is_primary":true,"text":[{"text":"    } else {","highlight_start":12,"highlight_end":13},{"text":"        #[cfg(not(roflol))]","highlight_start":1,"highlight_end":28},{"text":"        if y == \"world\" {","highlight_start":1,"highlight_end":26},{"text":"            println!(\"world!\")","highlight_start":1,"highlight_end":31},{"text":"        }","highlight_start":1,"highlight_end":10},{"text":"    }","highlight_start":1,"highlight_end":6}],"label":null,"suggested_replacement":"if y == \"world\" {\n        println!(\"world!\")\n    }","suggestion_applicability":"MachineApplicable","expansion":null}],"children":[],"rendered":null}],"rendered":"error: this `else { if .. }` block can be collapsed\n  --> tests/ui/collapsible_else_if.rs:85:12\n   |\nLL |       } else {\n   |  ____________^\nLL | |         #[cfg(not(roflol))]\nLL | |         if y == \"world\" {\nLL | |             println!(\"world!\")\nLL | |         }\nLL | |     }\n   | |_____^\n   |\nhelp: collapse nested if block\n   |\nLL |     } else if y == \"world\" {\nLL |         println!(\"world!\")\nLL |     }\n   |\n\n"}

------------------------------------------

diff of stderr:
diff of stderr:

 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:9:5
    |
 LL | /     if x == "hello" {
 LL | |         if y == "world" {
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
    = note: `-D clippy::collapsible-if` implied by `-D warnings`
 help: collapse nested if block
    |
 LL |     if x == "hello" && y == "world" {
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:15:5
    |
 LL | /     if x == "hello" || x == "world" {
 LL | |         if y == "world" || y == "hello" {
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if (x == "hello" || x == "world") && (y == "world" || y == "hello") {
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:21:5
    |
 LL | /     if x == "hello" && x == "world" {
 LL | |         if y == "world" || y == "hello" {
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if x == "hello" && x == "world" && (y == "world" || y == "hello") {
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:27:5
    |
 LL | /     if x == "hello" || x == "world" {
 LL | |         if y == "world" && y == "hello" {
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if (x == "hello" || x == "world") && y == "world" && y == "hello" {
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:33:5
    |
 LL | /     if x == "hello" && x == "world" {
 LL | |         if y == "world" && y == "hello" {
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if x == "hello" && x == "world" && y == "world" && y == "hello" {
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:39:5
    |
 LL | /     if 42 == 1337 {
 LL | |         if 'a' != 'A' {
 LL | |             println!("world!")
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if 42 == 1337 && 'a' != 'A' {
 LL |         println!("world!")
    |
 
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:95:5
   --> $DIR/collapsible_if.rs:95:5
    |
 LL | /     if x == "hello" {
 LL | |         if y == "world" { // Collapsible
 LL | |             println!("Hello world!");
 LL | |         }
 LL | |     }
    |
    |
 help: collapse nested if block
    |
 LL |     if x == "hello" && y == "world" { // Collapsible
 LL |     }
    |
 
 error: this `if` statement can be collapsed
 error: this `if` statement can be collapsed
   --> $DIR/collapsible_if.rs:154:5
    |
 LL | /     if matches!(true, true) {
 LL | |         if matches!(true, true) {}
 LL | |     }
    | |_____^ help: collapse nested if block: `if matches!(true, true) && matches!(true, true) {}`
-error: aborting due to 8 previous errors
+error: this `if` statement can be collapsed
+  --> $DIR/collapsible_if.rs:158:5
+   |
+   |
+LL | /     if true {
+LL | |         #[cfg(not(teehee))]
+LL | |         if true {
+LL | |             println!("Hello world!");
+LL | |         }
+LL | |     }
+   |
+   |
+help: collapse nested if block
+LL |     if true && true {
+LL |         println!("Hello world!");
+LL |     }
+   |
+   |
+
+error: aborting due to 9 previous errors
 
 

The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/build/clippy-4cfe8e74ca7ddba1/out/test_build_base/collapsible_if.stage-id.stderr

 // run-rustfix
 // run-rustfix
 #![allow(clippy::assertions_on_constants)]
 
 #[rustfmt::skip]
 #[warn(clippy::collapsible_if)]
error: test failed, to rerun pass '--test compile-test'
     let x = "hello";
     let x = "hello";
     let y = "world";
     if x == "hello" && y == "world" {
     }
 
 
     if (x == "hello" || x == "world") && (y == "world" || y == "hello") {
     }
 
 
     if x == "hello" && x == "world" && (y == "world" || y == "hello") {
     }
 
 
     if (x == "hello" || x == "world") && y == "world" && y == "hello" {
     }
 
 
     if x == "hello" && x == "world" && y == "world" && y == "hello" {
     }
 
 
     if 42 == 1337 && 'a' != 'A' {
         println!("world!")
 
 
     // Works because any if with an else statement cannot be collapsed.
     if x == "hello" {
         if y == "world" {
         }
     } else {
     } else {
         println!("Not Hello world");
 
 
     if x == "hello" {
         if y == "world" {
         } else {
         } else {
             println!("Hello something else");
     }
 
 
     if x == "hello" {
         print!("Hello ");

bors · 2021-04-13T22:25:44Z

💔 Test failed - checks-actions

petrochenkov · 2021-04-14T07:18:11Z

Some clippy test failures.
It's possible that some lint logic relies on cfg(TRUE)s staying there.

Aaron1011 · 2021-04-15T16:39:09Z

@petrochenkov: You're correct - the following test:

rust/src/tools/clippy/tests/ui/crashes/returns.rs

Lines 3 to 9 in f1ca558

    
           #[deny(warnings)] 
        
           fn cfg_return() -> i32 { 
        
               #[cfg(unix)] 
        
               return 1; 
        
               #[cfg(not(unix))] 
        
               return 2; 
        
           }

tests that lints are not emitted, due to this check:

rust/src/tools/clippy/clippy_lints/src/returns.rs

Lines 171 to 182 in f1ca558

    
           fn check_final_expr<'tcx>( 
        
               cx: &LateContext<'tcx>, 
        
               expr: &'tcx Expr<'tcx>, 
        
               span: Option<Span>, 
        
               replacement: RetReplacement, 
        
           ) { 
        
               match expr.kind { 
        
                   // simple return is always "bad" 
        
                   ExprKind::Ret(ref inner) => { 
        
                       // allow `#[cfg(a)] return a; #[cfg(b)] return b;` 
        
                       let attrs = cx.tcx.hir().attrs(expr.hir_id); 
        
                       if !attrs.iter().any(attr_is_cfg) {

cc @rust-lang/clippy: This PR removes passing #[cfg] attributes from attribute targets. It looks like this will be a problem for all 'late' lint passes, which run after macro expansion (and therefore cfg-expansion).

Possible solutions:

Give up on this PR. This seems unfortunate, as I believe @petrochenkov had stated that the current behavior is an undocumented bug.
Create a global table mapping NodeIds to stripped passing #[cfg] attributes, which is only build when Clippy is running. This seems kind of hacky, but would allow us to land this PR without breaking Clippy.

petrochenkov · 2021-04-15T17:50:08Z

The thing I proposed in #79341 (which also wants to use cfg in a way similar to clippy) is to expand #[cfg(TRUE_PREDICATE)] to #[rustc_cfg_trace(TRUE_PREDICATE)], but only when it's enabled by some option in rustc_interface/rustc_driver. Then rustdoc and clippy will enable it, and rustc will not.
(rustc_cfg_trace will be an eternally unstable inert built-in attribute gated with feature(rustc_attrs).)

Aaron1011 · 2021-04-15T18:13:45Z

Would that rustc_cfg_trace attr show up in the input to derive macros? It probably wouldn't break anything, but I think it be good for proc-macrod to receive exactly the same input across Clippy and rustc.

petrochenkov · 2021-04-15T18:28:48Z

Would that rustc_cfg_trace attr show up in the input to derive macros?

Yes.

I think it be good for proc-macrod to receive exactly the same input across Clippy and rustc

The input may already differ due to #[cfg(doc)]? (Well, in case of rustdoc and rustc, not clippy.)
I don't think it matters in practice, if the removal of #[cfg(TRUE)]s didn't break anything (most probably, although I still didn't investigate all regressions in #83354), then rustc_cfg_trace won't change much either.

camsteffen · 2021-04-15T18:32:58Z

Is there any need to be able to observe TRUE_PREDICATE? Maybe we just need a sort of boolean flag for nodes that are "cfg-conditional".

Aaron1011 · 2021-04-15T18:41:00Z

I don't think it matters in practice, if the removal of #[cfg(TRUE)]s didn't break anything (most probably, although I still didn't investigate all regressions in #83354), then rustc_cfg_trace won't change much either.

I'm not that concerned about breaking existing crates. My main concern is that this could lead to confusing scenarios where running Clippy produces very different output than running rustc normally.

When I was patching crates for the various None-delimited group cases, there were several crates that checked for the token Option, and produced different code depending on whether or not it existed. When we started wrapping the Option token on a None-delimited group, the macro still parsed the input successfully, but ended up causing a completely different trait-related error.

I think something very similar could end up happening here - a proc-macro crate (for whatever reason) starts checking for #[cfg], and does something different depending on whether or not it exists. I don't think this is likely, but it would probably be very frustrating if it did happen.

yaahc · 2021-04-15T18:52:18Z

Especially with export RUSTC_WORKSPACE_WRAPPER=clippy-driver being a thing, I too am worried about this becoming a problem in practice.

camelid · 2021-08-29T00:38:14Z

triage: Any updates?

estebank · 2021-08-29T10:23:36Z

src/test/ui/rust-2018/issue-54400-unused-extern-crate-attr-span.stderr

+LL | extern crate edition_lint_paths;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it


This is a regression, this test won't (shouldn't?) pass with the span change.

I think it actually will pass with the span change, but it is still a regression. The reason it will pass is because the #[cfg] will end up being applied to the fn main() {} that is directly below (from a whitespace- and comment-ignoring point of view) this extern crate.

Probably the test should be changed to use #![crate_type = "lib"] and to remove fn main() {} so it will fail if there's a future regression like this.

JohnCSimon · 2021-12-05T00:14:57Z

@Aaron1011
triage: Any updates?

expand: Pick `cfg`s and `cfg_attrs` one by one, like other attributes This is a rebase of rust-lang#83354, but without any language-changing parts ~(except for rust-lang#84110, i.e. the attribute expansion order is the same. This is a pre-requisite for any other changes making cfg attributes closer to regular macro attributes - Possibly changing their expansion order (rust-lang#83331) - Keeping macro backtraces for cfg attributes, or otherwise making them visible after expansion without keeping them in place literally (rust-lang#84110). Two exceptions to the "one by one" behavior are: - cfgs eagerly expanded by `derive` and `cfg_eval`, they are still expanded in a batch, that's by design. - cfgs at the crate root, they are currently expanded not during the main expansion pass, but before that, during `#![feature]` collection. I'll try to disentangle that logic later in a separate PR. r? `@Aaron1011`

Dylan-DPC · 2022-05-13T11:15:01Z

Closing this as inactive

rust-highfive assigned estebank Apr 11, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2021

rust-highfive assigned petrochenkov and unassigned estebank Apr 11, 2021

jonas-schievink mentioned this pull request Apr 11, 2021

Custom derive attribute stripping is insufficient rust-lang/rust-analyzer#8434

Closed

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2021

Aaron1011 added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 13, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 13, 2021

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 14, 2021

crlf0710 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2021

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 5, 2021

bstrie added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 21, 2021

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 9, 2021

estebank reviewed Aug 29, 2021

View reviewed changes

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 20, 2021

camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2021

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 14, 2021

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 31, 2021

petrochenkov mentioned this pull request Jan 1, 2022

expand: Pick cfgs and cfg_attrs one by one, like other attributes #92473

Merged

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 6, 2022

JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 6, 2022

Dylan-DPC closed this May 13, 2022

Dylan-DPC added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 13, 2022

		LL \| extern crate edition_lint_paths;
		\| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it

Uh oh!

Conversation

Aaron1011 commented Apr 11, 2021

Uh oh!

rust-highfive commented Apr 11, 2021

Uh oh!

Aaron1011 commented Apr 11, 2021

Uh oh!

petrochenkov commented Apr 13, 2021

Uh oh!

bors commented Apr 13, 2021

Uh oh!

petrochenkov commented Apr 13, 2021

Uh oh!

bors commented Apr 13, 2021

Uh oh!

rust-log-analyzer commented Apr 13, 2021

Uh oh!

bors commented Apr 13, 2021

Uh oh!

petrochenkov commented Apr 14, 2021

Uh oh!

Aaron1011 commented Apr 15, 2021

Uh oh!

petrochenkov commented Apr 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Aaron1011 commented Apr 15, 2021

Uh oh!

petrochenkov commented Apr 15, 2021

Uh oh!

camsteffen commented Apr 15, 2021

Uh oh!

Aaron1011 commented Apr 15, 2021

Uh oh!

yaahc commented Apr 15, 2021

Uh oh!

camelid commented Aug 29, 2021

Uh oh!

estebank Aug 29, 2021

Choose a reason for hiding this comment

Uh oh!

camelid Aug 30, 2021

Choose a reason for hiding this comment

Uh oh!

JohnCSimon commented Dec 5, 2021

Uh oh!

Dylan-DPC commented May 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants

petrochenkov commented Apr 15, 2021 •

edited

Loading