rustc_lint: address latent TODO#118017
Conversation
|
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
compiler/rustc_lint/src/lib.rs
Outdated
| #![feature(min_specialization)] | ||
| #![feature(never_type)] | ||
| #![feature(rustc_attrs)] | ||
| #![feature(trait_upcasting)] |
There was a problem hiding this comment.
Is the feature stable enough to use?
There was a problem hiding this comment.
Seems it's headed for stabilization. See #65991 (comment) and #65991 (comment). FCP ended yesterday.
There was a problem hiding this comment.
Yes, and also, trait_upcasting is used elsewhere in the compiler if I recall correctly
| let store: &dyn Any = &**store; | ||
| store.downcast_ref().unwrap() | ||
| } | ||
| } |
There was a problem hiding this comment.
Should we keep unerased_lint_store, and prefer a comment on Session::lint_store?
There was a problem hiding this comment.
I don't understand how to answer this question. Can you suggest why that's preferable? I've explained this change in the commit message - did you read it?
There was a problem hiding this comment.
A comment is usually the simplest way to make something more discoverable. Accessing the unerased lint store is exceptional, so I'm not sure this warrants a new trait & impl.
There was a problem hiding this comment.
How is it exceptional? The lint store is never accessed without unerasing because nothing can be done with the erased type.
There was a problem hiding this comment.
I have a mild preference for the original function:
- A trait seems like overkill, when there is a single method implemented for a single type. The trait suggests generality, i.e. there might be other types that implement the trait.
- The "because
Session::lint_storeis type-erased" comment is useful, and should be preserved even if the trait is kept.
There was a problem hiding this comment.
Removed the last commit.
|
☔ The latest upstream changes (presumably #118143) made this pull request unmergeable. Please resolve the merge conflicts. |
tamird
force-pushed
the
better-safety
branch
from
4cb4d3b to
4ded7ee
Compare
`Option::unwrap` is called on the next line.
tamird
force-pushed
the
better-safety
branch
from
4ded7ee to
e5139e6
Compare
|
r? @nnethercote |
tamird
force-pushed
the
better-safety
branch
from
e5139e6 to
55393b6
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
rustc_lint: address latent TODO See individual commits.
…llaumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#116446 (Yeet `mir::Const::from_anon_const`) - rust-lang#117871 (remove unused pub fns) - rust-lang#118017 (rustc_lint: address latent TODO) - rust-lang#118199 (Remove `HirId` from `QPath::LangItem`) - rust-lang#118272 (resolve: Avoid clones of `MacroData`) - rust-lang#118291 (rustdoc-search: clean up some DOM code) Failed merges: - rust-lang#118201 (Miscellaneous `ObligationCauseCode` cleanups) - rust-lang#118256 (rustc: `hir().local_def_id_to_hir_id()` -> `tcx.local_def_id_to_hir_id()` cleanup) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#118017 - tamird:better-safety, r=cjgillot rustc_lint: address latent TODO See individual commits.
7 participants
See individual commits.