dead-code lint: say "constructed" for structs #52332

zackmdavis · 2018-07-13T03:51:01Z

Respectively.

This is a sequel to November 2017's #46103 / 1a9dc2e. It had been
reported (more than once—at least #19140, #44083, and #44565) that the
"never used" language was confusing for enum variants that were "used"
as match patterns, so the wording was changed to say never "constructed"
specifically for enum variants. More recently, the same issue was raised
for structs (#52325). It seems consistent to say "constructed" here,
too, for the same reasons.

While we're here, we can also use more specific word "called" for unused
functions and methods. (We declined to do this in #46103, but the
rationale given in the commit message doesn't actually make sense.)

This resolves #52325.

rust-highfive · 2018-07-13T03:51:12Z

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

rust-highfive · 2018-07-13T04:45:17Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:start:test_ui-fulldeps
Check compiletest suite=ui-fulldeps mode=ui (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:51:42] 
[00:51:42] running 21 tests
[00:51:58] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:51:58] ......F..............
[00:51:58] 
[00:51:58] ---- [ui] ui-fulldeps/lint-plugin-cmdline-allow.rs stdout ----
[00:51:58] diff of stderr:
[00:51:58] 
[00:51:58] 
[00:51:58] - warning: function is never used: `lintme`
[00:51:58] + warning: function is never called: `lintme`
[00:51:58] 3    |
[00:51:58] 3    |
[00:51:58] 4 LL | fn lintme() { }
[00:51:58] 
[00:51:58] The actual stderr differed from the expected stderr.
[00:51:58] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/lint-plugin-cmdline-allow/lint-plugin-cmdline-allow.stderr
[00:51:58] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/lint-plugin-cmdline-allow/lint-plugin-cmdline-allow.stderr
[00:51:58] To update references, rerun the tests and pass the `--bless` flag
[00:51:58] To only update this specific test, also pass `--test-args lint-plugin-cmdline-allow.rs`
[00:51:58] error: 1 errors occurred comparing output.
[00:51:58] status: exit code: 0
[00:51:58] status: exit code: 0
[00:51:58] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/lint-plugin-cmdline-allow/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-A" "test-lint" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps/lint-plugin-cmdline-allow/auxiliary" "-A" "unused"
[00:51:58] ------------------------------------------
[00:51:58] 
[00:51:58] ------------------------------------------
[00:51:58] stderr:
[00:51:58] stderr:
[00:51:58] ------------------------------------------
[00:51:58] {"message":"function is never called: `lintme`","code":{"code":"dead_code","explanation":null},"level":"warning","spans":[{"file_name":"/checkout/src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs","byte_start":628,"byte_end":639,"line_start":20,"line_end":20,"column_start":1,"column_end":12,"is_primary":true,"text":[{"text":"fn lintme() { }","highlight_start":1,"highlight_end":12}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"lint level defined here","code":null,"level":"note","spans":[{"file_name":"/checkout/src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs","byte_start":589,"byte_end":595,"line_start":17,"line_end":17,"column_start":9,"column_end":15,"is_primary":true,"text":[{"text":"#![warn(unused)]","highlight_start":9,"highlight_end":15}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":null},{"message":"#[warn(dead_code)] implied by #[warn(unused)]","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"warning: function is never called: `lintme`\n  --> /checkout/src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs:20:1\n   |\nLL | fn lintme() { }\n   | ^^^^^^^^^^^\n   |\nnote: lint level defined here\n  --> /checkout/src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs:17:9\n   |\nLL | #![warn(unused)]\n   |         ^^^^^^\n   = note: #[warn(dead_code)] implied by #[warn(unused)]\n\n"}
[00:51:58] ------------------------------------------
[00:51:58] 
[00:51:58] thread '[ui] ui-fulldeps/lint-plugin-cmdline-allow.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3139:9
[00:51:58] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:51:58] test result: FAILED. 20 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
[00:51:58] 
[00:51:58] 
[00:51:58] 
[00:51:58] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui-fulldeps" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui-fulldeps" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-5.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "5.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:51:58] 
[00:51:58] 
[00:51:58] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:51:58] Build completed unsuccessfully in 0:10:07
[00:51:58] Build completed unsuccessfully in 0:10:07
[00:51:58] make: *** [check] Error 1
[00:51:58] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:28c9e81c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

pnkfelix · 2018-07-13T12:40:22Z

I agree with this PR's change of "constructed" instead of "used".

As for the broader change... I didn't mind the previous terminology of "use" instead of where it says "call" here, but I don't particularly care either way. If people like "call" then fine. I guess I'll just let this land and if someone complains we can change it back.

pnkfelix · 2018-07-13T12:40:58Z

@bors r+

bors · 2018-07-13T12:40:59Z

📌 Commit 8cc4fd6 has been approved by pnkfelix

…y_2_electric_boogaloo, r=pnkfelix dead-code lint: say "constructed", "called" for structs, functions Respectively. This is a sequel to November 2017's rust-lang#46103 / 1a9dc2e. It had been reported (more than once—at least rust-lang#19140, rust-lang#44083, and rust-lang#44565) that the "never used" language was confusing for enum variants that were "used" as match patterns, so the wording was changed to say never "constructed" specifically for enum variants. More recently, the same issue was raised for structs (rust-lang#52325). It seems consistent to say "constructed" here, too, for the same reasons. While we're here, we can also use more specific word "called" for unused functions and methods. (We declined to do this in rust-lang#46103, but the rationale given in the commit message doesn't actually make sense.) This resolves rust-lang#52325.

@ghost

Rollup of 16 pull requests Successful merges: - #51962 (Provide llvm-strip in llvm-tools component) - #52003 (Implement `Option::replace` in the core library) - #52156 (Update std::ascii::ASCIIExt deprecation notes) - #52242 (NLL: Suggest `ref mut` and `&mut self`) - #52244 (Don't display default generic parameters in diagnostics that compare types) - #52290 (Deny bare trait objects in src/librustc_save_analysis) - #52293 (Deny bare trait objects in librustc_typeck) - #52299 (Deny bare trait objects in src/libserialize) - #52300 (Deny bare trait objects in librustc_target and libtest) - #52302 (Deny bare trait objects in the rest of rust) - #52310 (Backport 1.27.1 release notes to master) - #52314 (Fix ICE when using a pointer cast as array size) - #52315 (Resolve FIXME(#27942)) - #52316 (task: remove wrong comments about non-existent LocalWake trait) - #52322 (Update llvm-rebuild-trigger in light of LLVM 7 upgrade) - #52332 (dead-code lint: say "constructed", "called" for structs, functions) Failed merges: r? @ghost

pietroalbini · 2018-07-13T17:56:13Z

@bors r-
This is breaking Cargo's test suite :(

pnkfelix · 2018-07-17T13:21:47Z

@zackmdavis the simplest solution might to be continue using the word "used" instead of "called" for functions, since it seems like cargo's test suite has that phrasing built into their current test suite.

(Otherwise I don't know how easy it would be to update cargo's test suite in sync with making the change to the wording proposed here.)

zackmdavis · 2018-07-17T16:09:44Z

the simplest solution might to be continue using the word "used" instead of "called" for functions

Yes, have to leave now, but I can revise this PR tonight. Updating Cargo's tests wouldn't be difficult, but I'm not actually super-passionate about "called"

zackmdavis · 2018-07-18T06:25:58Z

tonight

(Okay, maybe not tonight tonight, but soon)

This is a sequel to November 2017's rust-lang#46103 / 1a9dc2e. It had been reported (more than once—at least rust-lang#19140, rust-lang#44083, and rust-lang#44565) that the "never used" language was confusing for enum variants that were "used" as match patterns, so the wording was changed to say never "constructed" specifically for enum variants. More recently, the same issue was raised for structs (rust-lang#52325). It seems consistent to say "constructed" here, too, for the same reasons. We considered using more specific word "called" for unused functions and methods (while we declined to do this in rust-lang#46103, the rationale given in the commit message doesn't actually make sense), but it turns out that Cargo's test suite expects the "never used" message, and maybe we don't care enough even to make a Cargo PR over such a petty and subjective wording change. This resolves rust-lang#52325.

zackmdavis · 2018-07-22T02:57:46Z

@pnkfelix updated 🏁

pietroalbini · 2018-07-30T16:53:00Z

Ping from triage @pnkfelix! This PR needs your review.

pnkfelix · 2018-07-31T00:08:49Z

@bors r+ rollup

bors · 2018-07-31T00:08:50Z

📌 Commit 6c50ee5 has been approved by pnkfelix

…y_2_electric_boogaloo, r=pnkfelix dead-code lint: say "constructed" for structs Respectively. This is a sequel to November 2017's rust-lang#46103 / 1a9dc2e. It had been reported (more than once—at least rust-lang#19140, rust-lang#44083, and rust-lang#44565) that the "never used" language was confusing for enum variants that were "used" as match patterns, so the wording was changed to say never "constructed" specifically for enum variants. More recently, the same issue was raised for structs (rust-lang#52325). It seems consistent to say "constructed" here, too, for the same reasons. ~~While we're here, we can also use more specific word "called" for unused functions and methods. (We declined to do this in rust-lang#46103, but the rationale given in the commit message doesn't actually make sense.)~~ This resolves rust-lang#52325.

@ghost

Rollup of 31 pull requests Successful merges: - #52332 (dead-code lint: say "constructed" for structs) - #52340 (Document From trait implementations for OsStr, OsString, CString, and CStr) - #52628 (Cleanup some rustdoc code) - #52732 (Remove unstable and deprecated APIs) - #52745 (Update clippy to latest master) - #52756 (rustc: Disallow machine applicability in foreign macros) - #52771 (Clarify thread::park semantics) - #52810 ([NLL] Don't make "fake" match variables mutable) - #52821 (pretty print for std::collections::vecdeque) - #52822 (Fix From<LocalWaker>) - #52824 (Fix -Wpessimizing-move warnings in rustllvm/PassWrapper) - #52831 (remove references to AUTHORS.txt file) - #52835 (Fix Alias intra doc ICE) - #52842 (update comment) - #52846 (Add timeout to use of `curl` in bootstrap.py.) - #52851 (Make the tool_lints actually usable) - #52853 (Improve bootstrap help on stages) - #52859 (Use Vec::extend in SmallVec::extend when applicable) - #52861 (Add targets for HermitCore (https://hermitcore.org) to the Rust compiler and port libstd to it.) - #52867 (releases.md: fix 2 typos) - #52870 (Implement Unpin for FutureObj and LocalFutureObj) - #52876 (run-pass/const-endianness: negate before to_le()) - #52878 (Fix wrong issue number in the test name) - #52883 (Include lifetime in mutability suggestion in NLL messages) - #52904 (NLL: sort diagnostics by span) - #52905 (Fix a typo in unsize.rs) - #52907 (NLL: On "cannot move out of type" error, print original before rewrite) - #52908 (Use SetLenOnDrop in Vec::truncate()) - #52914 (Only run the sparc-abi test on sparc) - #52918 (Backport 1.27.2 release notes) - #52929 (Update compatibility note for 1.28.0 to be correct) Failed merges: - #52758 (Cleanup for librustc::session) - #52799 (Use BitVector for global sets of AttrId) r? @ghost

kennytm · 2018-08-01T04:44:24Z

@bors r- rollup-

RLS's test cases need to be updated as well. It contained 3 tests which still expects the old error messages. See #52931 (comment)

zackmdavis · 2018-08-05T00:05:25Z

↑ (This was sufficiently demoralizing and #52325 is sufficiently low-priority that I'm just going to close this for now to get it out of the way.)

Mark-Simulacrum · 2018-08-05T00:07:11Z

Hm, I don't quite follow -- why are we blocking this on RLS test cases? We should be able to land things in tree even if they break RLS, no?

kennytm · 2018-08-05T01:54:04Z

This PR was r+'ed during the beta week unfortunately 🙁.

Mark-Simulacrum · 2018-08-05T03:04:15Z

@bors r=pnkfelix

We should see if there's something we can do to provide better feedback for that case.

bors · 2018-08-05T03:04:16Z

📌 Commit 6c50ee5 has been approved by pnkfelix

kennytm · 2018-08-05T05:17:13Z

@bors rollup since we're outside the beta week now.

bors · 2018-08-06T07:02:19Z

⌛ Testing commit 6c50ee5 with merge 7c98d2e...

…c_boogaloo, r=pnkfelix dead-code lint: say "constructed" for structs Respectively. This is a sequel to November 2017's #46103 / 1a9dc2e. It had been reported (more than once—at least #19140, #44083, and #44565) that the "never used" language was confusing for enum variants that were "used" as match patterns, so the wording was changed to say never "constructed" specifically for enum variants. More recently, the same issue was raised for structs (#52325). It seems consistent to say "constructed" here, too, for the same reasons. ~~While we're here, we can also use more specific word "called" for unused functions and methods. (We declined to do this in #46103, but the rationale given in the commit message doesn't actually make sense.)~~ This resolves #52325.

bors · 2018-08-06T09:11:19Z

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing 7c98d2e to master...

rust-highfive · 2018-08-06T09:12:46Z

📣 Toolstate changed by #52332!

Tested on commit 7c98d2e.
Direct link to PR: #52332

💔 rls on windows: test-pass → test-fail (cc @nrc, @rust-lang/infra).
💔 rls on linux: test-pass → test-fail (cc @nrc, @rust-lang/infra).

@nrc

Tested on commit rust-lang/rust@7c98d2e. Direct link to PR: <rust-lang/rust#52332> 💔 rls on windows: test-pass → test-fail (cc @nrc, @rust-lang/infra). 💔 rls on linux: test-pass → test-fail (cc @nrc, @rust-lang/infra).

@kennytm

Update RLS, Rustfmt, and Clippy Fixes bustage due to #52332 r? @kennytm

rust-highfive assigned pnkfelix Jul 13, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 13, 2018

zackmdavis mentioned this pull request Jul 13, 2018

Incorrect 'never used' warning for struct when only use is in pattern match #52325

Closed

zackmdavis force-pushed the dead_code_lint_should_say_2_electric_boogaloo branch from acccb8d to 8cc4fd6 Compare July 13, 2018 04:49

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 Jul 13, 2018

Mark-Simulacrum mentioned this pull request Jul 13, 2018

Rollup of 16 pull requests #52344

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 13, 2018

zackmdavis force-pushed the dead_code_lint_should_say_2_electric_boogaloo branch from 8cc4fd6 to 6c50ee5 Compare July 22, 2018 01:38

zackmdavis changed the title ~~dead-code lint: say "constructed", "called" for structs, functions~~ dead-code lint: say "constructed" for structs Jul 22, 2018

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

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 31, 2018

Mark-Simulacrum mentioned this pull request Aug 1, 2018

Rollup of 31 pull requests #52931

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 1, 2018

zackmdavis closed this Aug 5, 2018

Mark-Simulacrum reopened this Aug 5, 2018

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 5, 2018

bors merged commit 6c50ee5 into rust-lang:master Aug 6, 2018

nrc added a commit to rust-lang/rls that referenced this pull request Aug 7, 2018

Fix tests (fallout from rust-lang/rust#52332)

6d4e0dc

nrc mentioned this pull request Aug 7, 2018

Update RLS, Rustfmt, and Clippy #53141

Merged

bors added a commit that referenced this pull request Aug 8, 2018

Auto merge of #53141 - nrc:update, r=nrc

52c785b

Update RLS, Rustfmt, and Clippy Fixes bustage due to #52332 r? @kennytm

pnkfelix mentioned this pull request Oct 8, 2018

[1.30 beta] Test suite of the jemalloc-ctl crate is failing #54478

Closed

zackmdavis mentioned this pull request Dec 12, 2018

False dead_code warning on struct pattern match #56750

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dead-code lint: say "constructed" for structs #52332

dead-code lint: say "constructed" for structs #52332

zackmdavis commented Jul 13, 2018 •

edited

Loading

rust-highfive commented Jul 13, 2018

rust-highfive commented Jul 13, 2018

pnkfelix commented Jul 13, 2018 •

edited

Loading

pnkfelix commented Jul 13, 2018

bors commented Jul 13, 2018

pietroalbini commented Jul 13, 2018

pnkfelix commented Jul 17, 2018

zackmdavis commented Jul 17, 2018

zackmdavis commented Jul 18, 2018

zackmdavis commented Jul 22, 2018

pietroalbini commented Jul 30, 2018

pnkfelix commented Jul 31, 2018

bors commented Jul 31, 2018

kennytm commented Aug 1, 2018

zackmdavis commented Aug 5, 2018

Mark-Simulacrum commented Aug 5, 2018

kennytm commented Aug 5, 2018

Mark-Simulacrum commented Aug 5, 2018

bors commented Aug 5, 2018

kennytm commented Aug 5, 2018

bors commented Aug 6, 2018

bors commented Aug 6, 2018

rust-highfive commented Aug 6, 2018

dead-code lint: say "constructed" for structs #52332

dead-code lint: say "constructed" for structs #52332

Conversation

zackmdavis commented Jul 13, 2018 • edited Loading

rust-highfive commented Jul 13, 2018

rust-highfive commented Jul 13, 2018

pnkfelix commented Jul 13, 2018 • edited Loading

pnkfelix commented Jul 13, 2018

bors commented Jul 13, 2018

pietroalbini commented Jul 13, 2018

pnkfelix commented Jul 17, 2018

zackmdavis commented Jul 17, 2018

zackmdavis commented Jul 18, 2018

zackmdavis commented Jul 22, 2018

pietroalbini commented Jul 30, 2018

pnkfelix commented Jul 31, 2018

bors commented Jul 31, 2018

kennytm commented Aug 1, 2018

zackmdavis commented Aug 5, 2018

Mark-Simulacrum commented Aug 5, 2018

kennytm commented Aug 5, 2018

Mark-Simulacrum commented Aug 5, 2018

bors commented Aug 5, 2018

kennytm commented Aug 5, 2018

bors commented Aug 6, 2018

bors commented Aug 6, 2018

rust-highfive commented Aug 6, 2018

zackmdavis commented Jul 13, 2018 •

edited

Loading

pnkfelix commented Jul 13, 2018 •

edited

Loading