Skip to content

Conversation

compiler-errors
Copy link
Member

Use fresh infer vars to guide inference along in type_changing_struct_update.

Fixes #96878

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 3, 2022
@rust-highfive
Copy link
Contributor

r? @michaelwoerister

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2022
@michaelwoerister
Copy link
Member

r? rust-lang/compiler (do we have an alias for the types team?)

compiler-errors added a commit to compiler-errors/highfive that referenced this pull request Jun 7, 2022
inspired by rust-lang/rust#97705 (comment)
> (do we have an alias for the types team?)

cc @rust-lang/types -- I only added the members of t-types who are already on the reviewer rotation, specifically excluding @nikomatsakis and @spastorino since they aren't currently on the rotation.

also add myself to diagnostics since i am a member of that wg, and move @cjgillot to the compiler team from the compiler team contributors grouping.
@estebank
Copy link
Contributor

r? @lcnr

I'm r=me, but adding someone on the types team.

@rust-highfive rust-highfive assigned lcnr and unassigned estebank Jun 10, 2022
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm r=me

there's one optional improvement i found, if you're interested in implementing that

self.tcx.mk_adt(*adt, fresh_substs),
|_| {},
);
let base_ty = self.shallow_resolve(base_ty);
match base_ty.kind() {
ty::Adt(base_adt, base_subs) if adt == base_adt => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you now bug! if that isn#t the case? or at least delay_span_bug if it's ty::Error? would safe us 8 columns of indentation ^^

Copy link
Member Author

@compiler-errors compiler-errors Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ironically, I don't think check_expr_has_type_or_error actually returns ty::Error when the expected type != the checked type of the input expr, hence us (still) needing to call report_mismatched_types on line before:1605/after:1624. I can easily check, though.

I will see if I can simplify this, though. It can at least be changed to a let-chain :)

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@compiler-errors
Copy link
Member Author

I was able to turn the redundant report_mismatched_types into a span_bug! and I removed the string comparison with "union".

@bors r=lcnr

@compiler-errors
Copy link
Member Author

wake up bors

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Jun 11, 2022

📌 Commit 5052911 has been approved by lcnr

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 11, 2022
@bors
Copy link
Collaborator

bors commented Jun 11, 2022

⌛ Testing commit 5052911 with merge e652caa...

@bors
Copy link
Collaborator

bors commented Jun 12, 2022

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing e652caa to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 12, 2022
@bors bors merged commit e652caa into rust-lang:master Jun 12, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 12, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e652caa): comparison url.

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)
N/A N/A 0
Regressions 😿
(secondary)
N/A N/A 0
Improvements 🎉
(primary)
-3.4% -3.4% 1
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) -3.4% -3.4% 1

Cycles

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regressions found
mean1 max count2
Regressions 😿
(primary)
3.4% 5.8% 8
Regressions 😿
(secondary)
3.1% 4.4% 3
Improvements 🎉
(primary)
N/A N/A 0
Improvements 🎉
(secondary)
N/A N/A 0
All 😿🎉 (primary) 3.4% 5.8% 8

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? `@nikomatsakis`
cc `@rust-lang/types`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? ``@nikomatsakis``
cc ``@rust-lang/types``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? ```@nikomatsakis```
cc ```@rust-lang/types```
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? ````@nikomatsakis````
cc ````@rust-lang/types````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? `````@nikomatsakis`````
cc `````@rust-lang/types`````
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2022
…_update_is_probably_complete, r=oli-obk

Make `type_changing_struct_update` no longer an incomplete feature

After rust-lang#97705, I don't see what would make it incomplete anymore. `check_expr_struct_fields` seems to now implement the RFC to the letter.

r? ``````@nikomatsakis``````
cc ``````@rust-lang/types``````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

type_changing_struct_update: regression in type inference
8 participants