Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause #145642
+32
−1
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
rustbot
added
S-waiting-on-review
lcnr
reviewed
Aug 20, 2025
lcnr
reviewed
Aug 20, 2025
| @@ -2878,7 +2878,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { | |||
| // we check if `TraitB` can be reachable from `S` | |||
| // to determine whether to note `TraitA` is sealed trait. | |||
| if let ty::Adt(adt, _) = ty.kind() { | |||
| let visibilities = tcx.effective_visibilities(()); | |||
| let visibilities = &tcx.resolutions(()).effective_visibilities; | |||
There was a problem hiding this comment.
okay 🤔 what does the effective_visibilities query actually do with the effective_visibilities computed by by nameres? I generally feel like it should have some more comments somewhere.
cc @petrochenkov maybe?
There was a problem hiding this comment.
I noticed that in versions prior to #143431, &tcx.resolutions(()).effective_visibilities was used in this way. As for what exactly it does, I'm not quite sure. At the time, I used this API as suggested.🤔
There was a problem hiding this comment.
okay 🤔 what does the
effective_visibilitiesquery actually do with theeffective_visibilitiescomputed by by nameres?
Actually calculates them.
Name resolutions can only fill the "directly public" and "exported" levels, based on modules, reexports and nominal visibility.
The query uses type information and calculates the remaining "reachable" levels too.
There was a problem hiding this comment.
Here we are only checking Level::Reexported so it's ok to use the incomplete effective visibilities from name resolver.
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
bors
added
S-waiting-on-author
lcnr
added
beta-nominated
…t while proving a where-clause Signed-off-by: xizheyin <[email protected]>
|
Added some comments for the test, waiting for CI green... |
|
@bors r=lcnr |
|
@xizheyin: 🔑 Insufficient privileges: Not in reviewers |
|
Unfortunately, bors did not recognize the delegate instruction. 🤔 |
|
oh fun @bors r+ rollup |
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Aug 20, 2025
Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause Partially fix rust-lang#145611, but we should do something make cycle in this situation ICE. Instead of using a query, call `&tcx.resolutions(()).effective_visibilities`. r? `@lcnr` cc `@compiler-errors`
bors
added a commit
that referenced
this pull request
Aug 20, 2025
Rollup of 12 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144443 (Make target pointer width in target json an integer) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145415 (std_detect: RISC-V: implement implication to "C") - #145573 (Add an experimental unsafe(force_target_feature) attribute.) - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145674 (Enable triagebot `[review-changes-since]` feature) Failed merges: - #145647 (miri subtree update) r? `@ghost` `@rustbot` modify labels: rollup
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Aug 20, 2025
Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause Partially fix rust-lang#145611, but we should do something make cycle in this situation ICE. Instead of using a query, call `&tcx.resolutions(()).effective_visibilities`. r? ``@lcnr`` cc ``@compiler-errors``
bors
added a commit
that referenced
this pull request
Aug 20, 2025
Rollup of 14 pull requests Successful merges: - #143383 (stabilize `const_array_each_ref`) - #144443 (Make target pointer width in target json an integer) - #144758 ([Doc] Add links to the various collections) - #144915 (Defer tail call ret ty equality to check_tail_calls) - #145137 (Consolidate panicking functions in `slice/index.rs`) - #145256 (Add new `--test-codegen-backend` bootstrap option) - #145297 (fix(debuginfo): handle false positives in overflow check) - #145415 (std_detect: RISC-V: implement implication to "C") - #145642 (Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause) - #145650 (Fix JS search scripts path) - #145654 (Download CI GCC into the correct directory) - #145662 (Enforce correct number of arguments for `"x86-interrupt"` functions) - #145674 (Enable triagebot `[review-changes-since]` feature) - #145678 (Fix typo in docstring) r? `@ghost` `@rustbot` modify labels: rollup
jhpratt
added a commit
to jhpratt/rust
that referenced
this pull request
Aug 21, 2025
Do not use effective_visibilities query for Adt types of a local trait while proving a where-clause Partially fix rust-lang#145611, but we should do something make cycle in this situation ICE. Instead of using a query, call `&tcx.resolutions(()).effective_visibilities`. r? ```@lcnr``` cc ```@compiler-errors```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially fix #145611, but we should do something make cycle in this situation ICE.
Instead of using a query, call
&tcx.resolutions(()).effective_visibilities.r? @lcnr
cc @compiler-errors