-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
make invalid_value lint a bit smarter around enums #102281
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
3a8b164
to
c19daa4
Compare
compiler/rustc_lint/src/builtin.rs
Outdated
let existing_variants = adt.variants().iter().filter(|v| v.fields.is_empty()).count(); | ||
existing_variants > 1 | ||
/// Determines whether the given type is inhabited. `None` means that we don't know. | ||
fn ty_inhabited(ty: Ty<'_>) -> Option<bool> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use tcx.type_uninhabited_from
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. Unfortunately the documentation is not helpful:
Computes the set of modules from which this type is visibly uninhabited. To check whether a type is uninhabited at all (not just from a given module), you could check whether the forest is empty.
Now I still don't know if this is an underapproximation or an overapproximation -- is this "definitely uninhabited" or "maybe uninhabited"? No idea which forest it is talking about.^^ 🌲
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type_uninhabited_from
means definitely uninhabited. References and arrays of unknown length are considered inhabited by this query.
The "forest" is the query's return type, I agree this is esoteric. tcx.type_uninhabited_from(param_env.and(ty)).is_empty()
means that ty
is possibly inhabited (opposite of definitely uninhabited).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I can use is_ty_uninhabited_from
as a nice wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and no then I need a module to check 'from'...
2e5628e
to
556e015
Compare
556e015
to
67fd09d
Compare
@bors r+ |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#101555 (Stabilize `#![feature(mixed_integer_ops)]`) - rust-lang#102253 (rustdoc: use CSS containment to speed up render) - rust-lang#102281 (make invalid_value lint a bit smarter around enums) - rust-lang#102284 (Structured suggestion for missing `mut`/`const` in raw pointer) - rust-lang#102330 (rustdoc: remove no-op CSS `.srclink { font-weight; font-size }`) - rust-lang#102337 (Avoid LLVM-deprecated `Optional::hasValue`) - rust-lang#102356 (session: remove now-unnecessary lint `#[allow]`s) - rust-lang#102367 (rustdoc: remove redundant `#help-button` CSS) - rust-lang#102369 (Fix search result colors) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #102043