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

deny(warnings) doesn't affect all warnings, only those generated through the lint system #21204

Open
kmcallister opened this issue Jan 15, 2015 · 15 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@kmcallister
Copy link
Contributor

No description provided.

@kmcallister kmcallister added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-diagnostics Area: Messages for errors, warnings, and lints labels Jan 15, 2015
@nagisa
Copy link
Member

nagisa commented Jan 15, 2015

   -D OPT, --deny OPT
          Set lint denied

@kmcallister kmcallister added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jan 15, 2015
@kmcallister
Copy link
Contributor Author

The documentation is correct. But the behavior is extremely surprising and is likely to cause major trouble. -D warnings doesn't error on all warnings, just the ones that are nicely documented and commonly encountered. More obscure messages that are hard-wired with span_warn will go un-noticed on a build server until the scenario they're trying to warn about occurs.

This shouldn't be too hard to fix; we just need to make -D warnings more special than it already is, by communicating that setting directly to the diagnostics printer.

@brson
Copy link
Contributor

brson commented Jan 26, 2015

There is code to make -A warnings as set on the command line apply to the warnings rustc emits, but even that only applies to the Session's diagnostics and not to the ParseSess's.

Perhaps SpanHandler just needs a callback to do arbitrary error transformation that can be hooked into the lint system.

@eddyb
Copy link
Member

eddyb commented Jan 29, 2015

Why even have warn and span_warn? There's very few uses of it, and they can probably all go through lints.

@kmcallister
Copy link
Contributor Author

That sounds good to me if it can be done.

@huonw
Copy link
Member

huonw commented Nov 18, 2015

This is still an issue, although there's still not many uses of warn/span_warn.

@nejucomo
Copy link

nejucomo commented Dec 6, 2015

Hi. I just ran into this issue for dead_code. I tried adding #[deny(warnings)] and when that didn't work, then I tried #[deny(dead_code)] which also has no apparent effect.

btw, I don't understand what a span_warn is, nor do I have any intuition about which warnings use the lint system or not, so I don't know if dead_code is one of these. I think this is a good argument for fixing this ticket since I doubt other users will be aware of these implementation distinctions.

@GuillaumeGomez
Copy link
Member

The problem still exists.

@nagisa
Copy link
Member

nagisa commented May 28, 2016

There’s a good reason to not support deny(warnings) at all from the stability standpoint. We want to add warnings at will without breaking users’ code and deny(warnings) makes that essentially an error.

@eddyb
Copy link
Member

eddyb commented May 28, 2016

@nagisa Didn't we solve that with --cap-lints and cargo setting it to "allow" for all dependencies?
We can add error-by-default lints without breaking users of libraries that trigger the lints.

@brson brson removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Jun 28, 2016
nejucomo added a commit to nejucomo/hashstore that referenced this issue Oct 9, 2016
@steveklabnik steveklabnik added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 9, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@steveklabnik
Copy link
Member

Triage:

#![deny(warnings)]

fn foo() {
    
}

at least now properly errors. I'm not sure if the compiler's warning APIs are the same as when this bug was filed, or if this issue persists. Many @Manishearth or someone else from clippy knows?

@Manishearth
Copy link
Member

I'm pretty sure I've seen deny(warnings) apply to non-lint warnings (this broke a rustup for firefox where we weren't able to #[allow()] our way out without allowing all lints), but I need to check

@Manishearth
Copy link
Member

Nope, this is still the case.

Check out:

#![deny(warnings)]


fn main() {
    let _a = "aa"_;
}

(non-lint warning taken from #42326 )

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority labels May 25, 2019
@iluuu1994
Copy link
Contributor

One problem with this one is that the example from @Manishearth "aa"_ is a lexer error. At that point the allow and deny attributes haven't been parsed. This example should compile fine but that would be hard to track in the lexer.

@torhovland
Copy link
Contributor

Given the insight from @iluuu1994, I don't think this issue should be tagged as easy.

@Dylan-DPC Dylan-DPC removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Oct 26, 2022
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 A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. P-low Low priority 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