Include ForeignItem when visiting types for WF check by PrestonFrom · Pull Request #98159 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation15 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Addresses Issue 95665 by including hir::Node::ForeignItem as a valid
type to visit in diagnostic_hir_wf_check.
Fixes #95665
Relevant to the compiler team, which will review and decide on the PR/issue.
label
(rust-highfive has picked a reviewer for you, use r? to override)
This fixes the issue originally reported, but I'm wondering if the other variants in hir::Node should be valid to visit. The comment on line 120 seemed like it would justify visiting any Ty that could be extracted, but I definitely don't have a complete understanding of what is happening. Any guidance would be appreciated!
The full list can be found by searching for places where WellFormedLoc::Ty is created.
Looks like all nodes except for foreign statics were already processed correctly.
HIR WF check is just about getting a more precise diagnostic. Can't we just let the match be None instead of panicking? This would give us a sub-optimal diagnostic, without any chance of ICE.
@cjgillot Apologies if this should be obvious, but I want to confirm that you mean the match on the ForeignItem kind and not the "parent" match on the WellFormedLoc.
@PrestonFrom in the cases the inner match returns None, the outer one will return the same result, won't it? In doubt: I mean replacing the bug! into None, not changing the other cases that already return Some.
@cjgillot The outer match currently uses bug! for the non-matching case, which I think is why it was suggested to use bug! here as well. So, I wasn't sure if you wanted to update both to return None or just the new addition.
r=me after squashing commits.
Addresses Issue 95665 by including hir::Node::ForeignItem as a valid
type to visit in diagnostic_hir_wf_check.
Fixes rust-lang#95665
📌 Commit f725b97 has been approved by petrochenkov
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: This is awaiting some action (such as code changes or more information) from the author.
labels
bors added a commit to rust-lang-ci/rust that referenced this pull request
Rollup of 8 pull requests
Successful merges:
- rust-lang#93080 (Implement
core::slice::IterMut::as_mut_sliceandimpl<T> AsMut<[T]> for IterMut<'_, T>) - rust-lang#94855 (Panic when advance_slices()'ing too far and update docs.)
- rust-lang#96609 (Add
{Arc, Rc}::downcast_unchecked) - rust-lang#96719 (Fix the generator example for
pin!()) - rust-lang#97149 (Windows:
CommandExt::async_pipes) - rust-lang#97150 (
Stdio::makes_pipe) - rust-lang#97837 (Document Rust's stance on
/proc/self/mem) - rust-lang#98159 (Include ForeignItem when visiting types for WF check)
Failed merges:
r? @ghost
@rustbot modify labels: rollup
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.