suggest_borrow_generic_arg: instantiate clauses properly
#133130
Conversation
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
I'm not certain that this is the right fix. If we're instantiating/substituting the wrong set of region variables into a type, then this is bound to happen with a generic type or const as well, right? Those cannot be erased.
I'd at least like to see an explanation for why this issue was happening before this fix; this feels like it's just masking the underlying issue which is that this code is handling generics kinda strangely in general.
There was a problem hiding this comment.
The real bug is that in clause_for_ref, you're substituting it once with the self type (ref_ty), then you're substituting it again in the call to predicate_must_hold_modulo_regions.
I don't think this really makes sense -- substituting it once should be sufficient, since you've already set up the generic args to have the right self type (new_args).
|
I hope that's enough info to guide you towards the right solution :) -- @rustbot author |
rustbot
added
S-waiting-on-author
Fixes issue 133118. This also modifies `tests/ui/moves/moved-value-on-as-ref-arg.rs` to have more useful bounds on the tests for suggestions to borrow `Borrow` and `BorrowMut` arguments. With its old tautological `T: BorrowMut<T>` bound, this fix would make it suggest a shared borrow for that argument.
|
This approach looks correct. Please update the description and title, then I can approve |
dianne
changed the title
suggest_borrow_generic_arg: erase region variables from moved_arg_ty soonersuggest_borrow_generic_arg: instantiate clauses properly
|
Done. Thank you! That helped. I think it reads much nicer now too. |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - rust-lang#132795 (Check `use<..>` in RPITIT for refinement) - rust-lang#132944 (add parentheses when unboxing suggestion needed) - rust-lang#132993 (Make rustc consider itself a stable compiler when `RUSTC_BOOTSTRAP=-1`) - rust-lang#133130 (`suggest_borrow_generic_arg`: instantiate clauses properly) - rust-lang#133133 (rustdoc-search: add standalone trailing `::` test) - rust-lang#133143 (Diagnostics for let mut in item context) - rust-lang#133147 (Fixup some test directives) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#133130 - dianne:fix-133118, r=compiler-errors `suggest_borrow_generic_arg`: instantiate clauses properly This simplifies and fixes the way `suggest_borrow_generic_arg` instantiates callees' predicates when testing them to see if a moved argument can instead be borrowed. Previously, it would ICE if the moved argument's type included a region variable, since it was getting passed to a call of `EarlyBinder::instantiate`. This makes the instantiation much more straightforward, which also fixes the ICE. Fixes rust-lang#133118 This also modifies `tests/ui/moves/moved-value-on-as-ref-arg.rs` to have more useful bounds on the tests for suggestions to borrow `Borrow` and `BorrowMut` arguments. With its old tautological `T: BorrowMut<T>` bound, this fix would make it suggest a shared borrow for that argument.
This simplifies and fixes the way
suggest_borrow_generic_arginstantiates callees' predicates when testing them to see if a moved argument can instead be borrowed. Previously, it would ICE if the moved argument's type included a region variable, since it was getting passed to a call ofEarlyBinder::instantiate. This makes the instantiation much more straightforward, which also fixes the ICE.Fixes #133118
This also modifies
tests/ui/moves/moved-value-on-as-ref-arg.rsto have more useful bounds on the tests for suggestions to borrowBorrowandBorrowMutarguments. With its old tautologicalT: BorrowMut<T>bound, this fix would make it suggest a shared borrow for that argument.