Skip to content
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

Handle rustc_mir_dataflow cases of rustc::potential_query_instability lint #131290

Conversation

ismailarilik
Copy link
Contributor

This PR removes #![allow(rustc::potential_query_instability)] occurrences from compiler/rustc_mir_dataflow/ and converts FxHash{Map,Set} types into FxIndex{Map,Set} to suppress lint errors.

A somewhat tracking issue: #84447

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 5, 2024
@cjgillot
Copy link
Contributor

cjgillot commented Oct 5, 2024

The current code is correct. It just needs a comment explaining why the #[allow(..)] is ok.

@compiler-errors
Copy link
Member

r? @cjgillot

@rustbot rustbot assigned cjgillot and unassigned compiler-errors Oct 5, 2024
@ismailarilik
Copy link
Contributor Author

So what can be the rationale; I can add it.

@cjgillot
Copy link
Contributor

The issue with FxHashMap/FxHashSet is that the iteration order is not deterministic. For instance, it may depend on the bit-width or the endianness of the host compiler. We want to avoid this. Multiple ways to achieve that:

  • use FxIndexMap/FxIndexSet which iteration order is deterministic;
  • sort the result of iteration;
  • the iteration order does not matter.

So the need is not to replace all containers, but rather to discriminate between cases that need replacement, from cases that don't need it.

@ismailarilik
Copy link
Contributor Author

Hmm. I don't have the knowledge to do that I guess.

@alex-semenyuk alex-semenyuk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 20, 2024
@alex-semenyuk
Copy link
Member

@ismailarilik
From wg-triage. Do you want to proceed with this PR?

@ismailarilik
Copy link
Contributor Author

@ismailarilik From wg-triage. Do you want to proceed with this PR?

Let me check the last time.

@ismailarilik ismailarilik force-pushed the handle-potential-query-instability-lint-for-rustc-mir-dataflow branch from 3c09e88 to 62c139e Compare November 20, 2024 14:53
@rustbot rustbot added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Nov 21, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 21, 2024

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

These commits modify compiler targets.
(See the Target Tier Policy.)

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in match lowering

cc @Nadrieril

Some changes occurred in match checking

cc @Nadrieril

HIR ty lowering was modified

cc @fmease

Some changes occurred in src/tools/cargo

cc @ehuss

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

These commits modify the library/Cargo.lock file. Unintentional changes to library/Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@ismailarilik ismailarilik marked this pull request as draft November 21, 2024 12:51
@ismailarilik
Copy link
Contributor Author

@rustbot label -WG-trait-system-refactor

@rustbot rustbot removed the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Nov 21, 2024
@ismailarilik ismailarilik force-pushed the handle-potential-query-instability-lint-for-rustc-mir-dataflow branch from 62c139e to 8adb4b3 Compare November 21, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants