-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
HashStable for rustc_feature::Features: stop hashing compile-time constant #132076
Conversation
LGTM, though I will wait for Mark to weigh in before approving, just in case. |
I think my reasoning was that the [bool] list might be the same if we reorder features in the list, but ideally would produce a different hash. However thinking more on it the compiler version etc. should also get hashed in if we care about that so it should be fine (as you point out). To some extent it looks like we've regressed on those goals anyway, we're hashing far more than bools these days:
But, still, this seems like a good change to make regardless. @bors r=nnethercote,Mark-Simulacrum |
Rollup of 10 pull requests Successful merges: - rust-lang#130225 (Rename Receiver -> LegacyReceiver) - rust-lang#131169 (Fix `target_vendor` in QNX Neutrino targets) - rust-lang#131623 (misc cleanups) - rust-lang#131756 (Deeply normalize `TypeTrace` when reporting type error in new solver) - rust-lang#131898 (minor `*dyn` cast cleanup) - rust-lang#131909 (Prevent overflowing enum cast from ICEing) - rust-lang#131930 (Don't allow test revisions that conflict with built in cfgs) - rust-lang#131956 (coverage: Pass coverage mappings to LLVM as separate structs) - rust-lang#132076 (HashStable for rustc_feature::Features: stop hashing compile-time constant) - rust-lang#132088 (Print safety correctly in extern static items) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132076 - RalfJung:feature-hashing, r=nnethercote,Mark-Simulacrum HashStable for rustc_feature::Features: stop hashing compile-time constant It seems like back in rust-lang@542bc75 this was added as "hash the boolean value of each lang feature", but then in rust-lang@1487bd6 this got split into first hashing a sequence of `bool`s (representing all the features) and then hashing all the feature names... but the list of feature names is a compile-time constant, so it seems entirely unnecessary to hash them? Cc `@Mark-Simulacrum` who wrote the second of the commits mentioned above. Cc `@nnethercote`
It seems like back in 542bc75 this was added as "hash the boolean value of each lang feature", but then in 1487bd6 this got split into first hashing a sequence of
bool
s (representing all the features) and then hashing all the feature names... but the list of feature names is a compile-time constant, so it seems entirely unnecessary to hash them?Cc @Mark-Simulacrum who wrote the second of the commits mentioned above.
Cc @nnethercote