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

Add sanity checking for query keys #68480

Open
Aaron1011 opened this issue Jan 23, 2020 · 1 comment
Open

Add sanity checking for query keys #68480

Aaron1011 opened this issue Jan 23, 2020 · 1 comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Aaron1011
Copy link
Member

We frequently see issues where a query parameter ends up containing something that it's not supposed to (an inference variable, a placeholder region, etc): #68477 and #64964 are recent examples.

Currently, incremental compilation must be enabled to see these crashes, since they only occur when we try to hash the 'bad' type. This presents a number of issues:

  1. The playground can't be used to reproduce them, since it (rightly) disables incremental compilation.
  2. We may miss these kinds of issues when invoking rustc directly (e.g. the ui test suite), since -C incremental=1 is usually not passed.
  3. The panic message isn't very helpful - in particular, it doesn't show the original type being hashed.

I think it would be useful to add a sanity_check method to Key, which would verify that the value is sane (e.g. no inference variances or placeholder regions) regardless of whether or not incremental compilation is enabled.

@jonas-schievink jonas-schievink added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 23, 2020
@bjorn3
Copy link
Member

bjorn3 commented Jan 26, 2020

Would hashing the keys with a hasher that does nothing when not in incremental mode fix this? Because it does nothing, the call to the hasher and most code inside StableHash::stable_hash will be optimized away, but any panic will still occur.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants