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

dead code lint to say "never constructed" for variants #46103

Conversation

zackmdavis
Copy link
Member

As reported in #19140, #44083, and #44565, some users were confused when
the dead-code lint reported an enum variant to be "unused" when it was
matched on (but not constructed). This wording change makes it clearer
that the lint is in fact checking for construction.

We continue to say "used" for all other items (it's tempting to say
"called" for functions and methods, but this turns out not to be
correct: functions can be passed as arguments and the dead-code lint
isn't special-casing that or anything).

Resolves #19140.

r? @pnkfelix

As reported in rust-lang#19140, rust-lang#44083, and rust-lang#44565, some users were confused when
the dead-code lint reported an enum variant to be "unused" when it was
matched on (but not constructed). This wording change makes it clearer
that the lint is in fact checking for construction.

We continue to say "used" for all other items (it's tempting to say
"called" for functions and methods, but this turns out not to be
correct: functions can be passed as arguments and the dead-code lint
isn't special-casing that or anything).

Resolves rust-lang#19140.
@nagisa
Copy link
Member

nagisa commented Nov 19, 2017

We continue to say "used" for all other items (it's tempting to say
"called" for functions and methods, but this turns out not to be
correct: functions can be passed as arguments and the dead-code lint
isn't special-casing that or anything).

Same also is true for enum variants though. Much like with functions:

enum Foo {
    A(usize),
}

fn main() {
    drop::<fn(usize)->Foo>(Foo::A);
}

will not warn, despite the enum variant never having been constructed. It might be worthwhile to find some other wording.

e.g. rather than saying that enum variant is never used, say that the enum variant constructor is never used or something?

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2017
@arielb1
Copy link
Contributor

arielb1 commented Nov 20, 2017

@nagisa

This is a static analysis, so it can never be 100% precise (see if false { Foo::A(0) }).

I think "variant is never constructed" is a fluent way of saying "variant constructor is never used".

@bors r+

@bors
Copy link
Contributor

bors commented Nov 20, 2017

📌 Commit 1a9dc2e has been approved by arielb1

@kennytm kennytm 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 Nov 21, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Nov 21, 2017
…y_never_constructed_for_variants, r=arielb1

dead code lint to say "never constructed" for variants

As reported in rust-lang#19140, rust-lang#44083, and rust-lang#44565, some users were confused when
the dead-code lint reported an enum variant to be "unused" when it was
matched on (but not constructed). This wording change makes it clearer
that the lint is in fact checking for construction.

We continue to say "used" for all other items (it's tempting to say
"called" for functions and methods, but this turns out not to be
correct: functions can be passed as arguments and the dead-code lint
isn't special-casing that or anything).

Resolves rust-lang#19140.

r? @pnkfelix
bors added a commit that referenced this pull request Nov 21, 2017
Rollup of 11 pull requests

- Successful merges: #45987, #46031, #46050, #46052, #46103, #46120, #46134, #46141, #46148, #46155, #46157
- Failed merges:
@bors bors merged commit 1a9dc2e into rust-lang:master Nov 21, 2017
@zackmdavis zackmdavis deleted the dead_code_lint_should_say_never_constructed_for_variants branch January 13, 2018 07:41
zackmdavis added a commit to zackmdavis/rust that referenced this pull request Jul 13, 2018
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.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 13, 2018
…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.
zackmdavis added a commit to zackmdavis/rust that referenced this pull request Jul 22, 2018
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.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 31, 2018
…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.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Aug 1, 2018
…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.
bors added a commit that referenced this pull request Aug 6, 2018
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants