fallback to resolve infer generics in type-changing-struct-update #102129

SparrowLii · 2022-09-22T08:32:35Z

Fixes #101970
When creating a struct with a base struct, we store the base struct's type which has infer generics. And then unify it with the created struct's generics through a function-body-scoped fallback during typeck.
cc #86555

rust-highfive · 2022-09-22T08:32:39Z

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

SparrowLii · 2022-09-22T08:33:52Z

cc @compiler-errors @nikomatsakis

bors · 2022-09-27T14:03:50Z

☔ The latest upstream changes (presumably #102306) made this pull request unmergeable. Please resolve the merge conflicts.

compiler-errors · 2022-09-29T16:47:01Z

compiler/rustc_infer/src/infer/base_struct.rs

+    let variances = tcx.variances_of(adt_ty.ty_adt_def().expect("not a adt ty!").did());
+
+    iter::zip(a_subst, b_subst).enumerate().for_each(|(i, (a, b))| {
+        let _arg: RelateResult<'tcx, GenericArg<'tcx>> =


Why are you throwing this result away?

The purpose of this code is to instantiate the uninstantiated infer generic created for base_expr in check_expr_struct_fields by calling the relate_generic_arg function. If Err is returned, the generic type has been instantiated by the type of base_expr itself and is not constrained by the created struct (constrained ones hav been checked in check_expr_struct_fields).

Perhaps we can have a more accurate approach, such as maintaining a non-constrained generic indexes list for base_expr.

Given that this is purely for diagnostics, this approach might be fine, but including what you wrote above as a comment would be welcome.

Given that this is purely for diagnostics

I'm confused by this comment. This isn't purely diagnostic, I thought? My understanding is that this function affects type inference -- notably, relate_generic_arg is also not an atomic operation, so even if it returns Err, it can still partially constrain inference variables inside of that.

I'm not convinced that this approach is completely sound. I would be comfortable with something more along the lines of:

such as maintaining a non-constrained generic indexes list for base_expr.

At least in that case it's easier to reason about when we're allowed to unify unconstrained inference variables and when we expect to raise errors.

bors · 2022-10-07T17:46:29Z

☔ The latest upstream changes (presumably #101632) made this pull request unmergeable. Please resolve the merge conflicts.

fee1-dead · 2022-11-04T14:00:54Z

Can we do a crater run to see if this will fix all breakages? On the issue the lang team expressed that this could be done via an edition change, but wouldn't it just work as good if this would resolve them? cc @scottmcm

estebank · 2022-11-10T22:40:25Z

@bors try

@craterbot run

craterbot · 2022-11-10T22:40:26Z

🚨 Error: missing start toolchain

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

bors · 2022-11-10T22:40:34Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout type-changing-struct-update (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self type-changing-struct-update --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging compiler/rustc_middle/src/ty/relate.rs
Auto-merging compiler/rustc_middle/src/ty/error.rs
Auto-merging compiler/rustc_middle/src/ty/context.rs
Auto-merging compiler/rustc_infer/src/infer/mod.rs
Auto-merging compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Auto-merging compiler/rustc_hir_typeck/src/expr.rs
CONFLICT (content): Merge conflict in compiler/rustc_hir_typeck/src/expr.rs
Auto-merging compiler/rustc_hir_typeck/src/demand.rs
CONFLICT (content): Merge conflict in compiler/rustc_hir_typeck/src/demand.rs
Auto-merging compiler/rustc_hir_analysis/src/check/mod.rs
CONFLICT (content): Merge conflict in compiler/rustc_hir_analysis/src/check/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

fee1-dead · 2022-11-11T00:50:14Z

@rustbot author

fee1-dead · 2022-11-11T06:06:23Z

@bors try

bors · 2022-11-11T06:06:34Z

⌛ Trying commit 61fd9d5 with merge cbe8532756bbbc4b7f32dda40d05981c8aeffd80...

SparrowLii · 2022-11-11T06:57:15Z

Just rebased the PR. We can have a crate run first

bors · 2022-11-11T08:46:58Z

☀️ Try build successful - checks-actions
Build commit: cbe8532756bbbc4b7f32dda40d05981c8aeffd80 (cbe8532756bbbc4b7f32dda40d05981c8aeffd80)

fee1-dead · 2022-11-11T12:57:18Z

@craterbot run

craterbot · 2022-11-11T12:57:26Z

👌 Experiment pr-102129 created and queued.
🤖 Automatically detected try build cbe8532756bbbc4b7f32dda40d05981c8aeffd80
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

fee1-dead · 2022-11-11T13:11:40Z

@craterbot abort

craterbot · 2022-11-11T13:11:42Z

🗑️ Experiment pr-102129 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

fee1-dead · 2022-11-11T13:11:55Z

@craterbot check

craterbot · 2022-11-11T13:11:59Z

👌 Experiment pr-102129 created and queued.
🤖 Automatically detected try build cbe8532756bbbc4b7f32dda40d05981c8aeffd80
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2022-11-11T15:50:52Z

🚧 Experiment pr-102129 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2022-11-12T13:38:31Z

🎉 Experiment pr-102129 is completed!
📊 82 regressed and 14 fixed (247865 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

estebank · 2022-11-22T01:08:00Z

I'm going to reroll, I don't have the bandwidth for this PR at the moment.

r? compiler

compiler-errors · 2022-11-22T01:11:58Z

r? @compiler-errors

This still seems to leave some regressions according to the crater run. This definitely needs further discussion discussion on the changes to inference here, and I still have some concerns (#102129 (comment)) on the exact implementation that we're doing here.

JohnCSimon · 2023-04-30T04:15:51Z

@SparrowLii
Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you are going to continue please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

rust-highfive assigned estebank Sep 22, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 22, 2022

compiler-errors reviewed Sep 29, 2022

View reviewed changes

SparrowLii force-pushed the type-changing-struct-update branch from 2ffc007 to ae5c596 Compare September 30, 2022 03:07

rustbot 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 11, 2022

SparrowLii added 4 commits November 11, 2022 11:32

demand_base_struct

b63aea5

simplify base_struct function

a04088b

optimize and add test

3bea6bc

enable the feature by default

61fd9d5

SparrowLii force-pushed the type-changing-struct-update branch from ae5c596 to 61fd9d5 Compare November 11, 2022 04:29

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 11, 2022

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 11, 2022

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 11, 2022

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Nov 12, 2022

rustbot assigned TaKO8Ki and unassigned estebank Nov 22, 2022

rustbot assigned compiler-errors and unassigned TaKO8Ki Nov 22, 2022

compiler-errors 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 25, 2022

JohnCSimon closed this Apr 30, 2023

rustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Apr 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fallback to resolve infer generics in type-changing-struct-update #102129

fallback to resolve infer generics in type-changing-struct-update #102129

SparrowLii commented Sep 22, 2022 •

edited

Loading

rust-highfive commented Sep 22, 2022

SparrowLii commented Sep 22, 2022

bors commented Sep 27, 2022

compiler-errors Sep 29, 2022

SparrowLii Sep 30, 2022

SparrowLii Sep 30, 2022

estebank Nov 10, 2022

compiler-errors Nov 10, 2022

bors commented Oct 7, 2022

fee1-dead commented Nov 4, 2022

estebank commented Nov 10, 2022

craterbot commented Nov 10, 2022

bors commented Nov 10, 2022

fee1-dead commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

bors commented Nov 11, 2022

SparrowLii commented Nov 11, 2022

bors commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

craterbot commented Nov 11, 2022

craterbot commented Nov 12, 2022

estebank commented Nov 22, 2022

compiler-errors commented Nov 22, 2022

JohnCSimon commented Apr 30, 2023

fallback to resolve infer generics in type-changing-struct-update #102129

fallback to resolve infer generics in type-changing-struct-update #102129

Conversation

SparrowLii commented Sep 22, 2022 • edited Loading

rust-highfive commented Sep 22, 2022

SparrowLii commented Sep 22, 2022

bors commented Sep 27, 2022

compiler-errors Sep 29, 2022

Choose a reason for hiding this comment

SparrowLii Sep 30, 2022

Choose a reason for hiding this comment

SparrowLii Sep 30, 2022

Choose a reason for hiding this comment

estebank Nov 10, 2022

Choose a reason for hiding this comment

compiler-errors Nov 10, 2022

Choose a reason for hiding this comment

bors commented Oct 7, 2022

fee1-dead commented Nov 4, 2022

estebank commented Nov 10, 2022

craterbot commented Nov 10, 2022

bors commented Nov 10, 2022

fee1-dead commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

bors commented Nov 11, 2022

SparrowLii commented Nov 11, 2022

bors commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

fee1-dead commented Nov 11, 2022

craterbot commented Nov 11, 2022

craterbot commented Nov 11, 2022

craterbot commented Nov 12, 2022

estebank commented Nov 22, 2022

compiler-errors commented Nov 22, 2022

JohnCSimon commented Apr 30, 2023

SparrowLii commented Sep 22, 2022 •

edited

Loading