-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
small normalization improvement #127570
small normalization improvement #127570
Conversation
@@ -81,7 +81,7 @@ bitflags::bitflags! { | |||
/// Does this have `Alias` or `ConstKind::Unevaluated`? | |||
/// | |||
/// Rephrased, could this term be normalized further? | |||
const HAS_ALIASES = TypeFlags::HAS_TY_PROJECTION.bits() | |||
const HAS_ALIAS = TypeFlags::HAS_TY_PROJECTION.bits() |
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.
👍
@@ -230,7 +230,7 @@ pub trait TypeVisitableExt<I: Interner>: TypeVisitable<I> { | |||
} | |||
|
|||
fn has_aliases(&self) -> 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.
fn has_aliases(&self) -> bool { | |
fn has_alias(&self) -> bool { |
too (my bad)
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.
interstingly, we do use the plural for the functions, e.g. we have has_inherent_projections
and has_opaque_types
even though the type flag is HAS_TY_OPAQUE
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.
let's leave this one
match reveal { | ||
Reveal::UserFacing => {} | ||
Reveal::All => flags |= ty::TypeFlags::HAS_TY_OPAQUE, | ||
Reveal::UserFacing => flags ^= ty::TypeFlags::HAS_TY_OPAQUE, |
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.
is it possible to & with the complement here? xor feels confusing to reason 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.
there's fn remove
👍
@bors r+ rollup |
Reveal::UserFacing => {} | ||
Reveal::All => flags |= ty::TypeFlags::HAS_TY_OPAQUE, | ||
Reveal::UserFacing => flags.remove(ty::TypeFlags::HAS_TY_OPAQUE), | ||
Reveal::All => {} |
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.
Logic here now inverted, is it ok?
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.
Ahh, HAS_TY_OPAQUE
wasn't in flags
before.
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0) - rust-lang#127094 (E0191 suggestion correction, inserts turbofish) - rust-lang#127554 ( do not run test where it cannot run) - rust-lang#127564 (Temporarily remove me from review rotation.) - rust-lang#127568 (instantiate higher ranked goals in candidate selection again) - rust-lang#127569 (Fix local download of Docker caches from CI) - rust-lang#127570 ( small normalization improvement) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#127570 - lcnr:normalize-cool, r=compiler-errors small normalization improvement r? `@compiler-errors`
r? @compiler-errors