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

Don't emit "value assigned to ... is never read" for each instance? #67548

Open
mqudsi opened this issue Dec 23, 2019 · 1 comment
Open

Don't emit "value assigned to ... is never read" for each instance? #67548

mqudsi opened this issue Dec 23, 2019 · 1 comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mqudsi
Copy link
Contributor

mqudsi commented Dec 23, 2019

Scrolling through warnings when compiling projects during the early development stages can be a bit of a chore, but there's not much the compiler can do about it as it doesn't know whether you just haven't gotten around to something or if you forgot about it. I typically add #![allow(unused_code)] and #![allow(dead_code)] to new projects and take them out when the codebase nears completion of v1, but it occurs to me that there's no real value in emitting repeated warnings for unused assignments of the same variable.

e.g.

image

There are different heuristics here depending on how important it is to generate all warnings in advance, including

  • a bloom filter for the variable (by ast path, not name) with the corresponding buckets filled when the first warning is emitted

This means code like

{
    let foo = bar;
    let foo = bar;
}

continues to emit two warnings as they are two different variables being unread, but this wouldn't emit two warnings:

{ 
    let mut foo = "bar";
    foo = "baz";
}

as we are deduplicating by variable and not by unread write so only the first assignment will generate the warning, which is probably ok since addressing that error by adding either of

{
    let mut foo = "bar";
    if (foo == "bar") { ... }
    foo = "baz";
}

or

{
    let mut foo = "bar";
    foo = "baz";
    if (foo == "bar") { ... }
}

will go back to generating the warning for the other instance of unused write.

  • actually tracking individual code paths for each variable and only warning for writes that are not provably exclusive:
{ 
    let mut foo = "bar";
    foo = "baz";
}

The above would continue to generate two warnings, but the following would generate only one:

let mut bar;
let foo = "hello";
match foo {
    "bar" => foo = "bar",
    "baz" => foo = "qux",
}

as no value was overwritten at any point and a single read would suffice to address both instances of unused writes.

Personally, I think the latter is overkill and might slow things down and increase memory usage in the compiler without much benefit. The former is both fast and easy (with no allocations and predetermined memory consumption based off the size of the bloom filter).

@mqudsi
Copy link
Contributor Author

mqudsi commented Dec 25, 2019

In the case of multiple unread writes to an immutable variable, there are no drawbacks to approach number one as they are necessarily exclusive.

@Alexendoo Alexendoo added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. labels Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants