Report all lints, even if other errors already occurred. #633
Comments
oli-obk
added
T-compiler
|
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed. cc @rust-lang/compiler @rust-lang/compiler-contributors |
|
@rustbot second |
rustbot
added
the
final-comment-period
|
@rustbot concern errors-inherently-higher-prioirty I'm worried about this change. Getting a wall of diagnostics can make it hard for users to tease out which things that they have to address to get their code to compile. At the very least, are we going to offer a flag that lets someone opt back into the old behavior, for those of us who prefer it? (or is your thinking "if you want the old behavior, you can first compile with |
well... I think we should improve diagnostics in other ways than having this hard and very arbitrary limit. Not all our lints are run in the generic lint machinery, but reported throughout the compilation. Those are already reported before the lints I enable here, so this PR just treats all lints equally. I would expect we'll figure out how to
|
|
For what it's worth, I strongly prefer only seeing errors as long as there are errors (but yes, denied lints should count as errors). There's time to fix warnings once the code compiles (first priority) and the program runs/tests pass (second priority). |
|
I find it really depends on the lint. Unused imports? I (almost certainly, but not necessarily) don't want to see them. But lints that are (more frequently) tied to bugs? I often do. It's hard to predict. Often times I am editing one part of the code and deliberately ignoring errors in other parts. I think anything that assumes people are going down the list of errors 1 by 1 is probably suspect, I tend to prefer a free-form approach, especially when I'm in an IDE. |
|
We already have one-off logic in diagnostics to silence redundant errors. We would have to extend those kind of checks to lints as well. Because that kind of silencing requires hand checking we can indeed take these kind of considerations when doing that. |
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
… r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
Rollup merge of rust-lang#120020 - oli-obk:long_const_eval_err_taint, r=compiler-errors Gracefully handle missing typeck information if typeck errored fixes rust-lang#116893 I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked rust-lang/compiler-team#633
apiraino
removed
the
final-comment-period
Proposal
Right now, if errors have occurred, we do not run our lints. This means that the user will see some errors, fix them, and once all of them are fixed, they will get new warnings and errors (the latter from
denylints).If we always run lints, this reduces surprises of the "I fixed all the errors, now I'm getting new ones"-kind
This is already implemented: rust-lang/rust#108813
Mentors or Reviewers
Reviewable by any compiler team member or compiler contributor.
Process
The main points of the Major Change Process are as follows:
@rustbot second.-C flag, then full team check-off is required.@rfcbot fcp mergeon either the MCP or the PR.You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
The text was updated successfully, but these errors were encountered: