Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

remove KEEP_IN_LOCAL_TCX flag #70860

Merged
merged 3 commits into from
Apr 9, 2020
Merged

remove KEEP_IN_LOCAL_TCX flag #70860

merged 3 commits into from
Apr 9, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Apr 6, 2020 •
edited
Loading

closes #70285

I did not rename needs_infer here as this complex enough as is.
Will probably open a followup for that.

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2020
@lcnr lcnr force-pushed the has_local_value branch from 0a132fc to 5e3374f Compare April 6, 2020 22:03
@lcnr lcnr force-pushed the has_local_value branch from 5e3374f to f4398c7 Compare April 6, 2020 22:06
eddyb
eddyb reviewed Apr 6, 2020
View reviewed changes
src/librustc_trait_selection/traits/mod.rs Outdated
Comment on lines 262 to 264
if predicates.has_local_value() {
if predicates.needs_infer() {
// FIXME: shouldn't we, you know, actually report an error here? or an ICE?
Err(ErrorReported)
Copy link
Member

@eddyb eddyb Apr 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

cc @nikomatsakis This is a scary FIXME comment 😮. Should we add a delay_span_bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a delay_span_bug. Should be a relevant error msg afaik

encountered inference variables after fully_resolve

@lcnr lcnr force-pushed the has_local_value branch 2 times, most recently from 9f5ebff to bc5d6ad Compare April 6, 2020 22:23
eddyb
eddyb reviewed Apr 6, 2020
View reviewed changes
src/librustc_middle/ty/relate.rs
Comment on lines -513 to 514
if !x.val.has_local_value() {
if !x.val.needs_infer() {
return x.eval(tcx, relation.param_env()).val;
Copy link
Member

Choose a reason for hiding this comment

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

cc @oli-obk @varkor This seems unnecessary, ty::Const::eval handles needs_infer itself.

eddyb
eddyb reviewed Apr 6, 2020
View reviewed changes
eddyb
eddyb approved these changes Apr 6, 2020
View reviewed changes
Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

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

LGTM

@eddyb
Copy link
Member

eddyb commented Apr 6, 2020

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Apr 6, 2020
@lcnr lcnr force-pushed the has_local_value branch 3 times, most recently from 1fadf31 to 3358803 Compare April 7, 2020 08:49
@nikomatsakis
Copy link
Contributor

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Apr 7, 2020

📌 Commit 3358803b44e40b97eb7f8c8e00ad00c8824ab73a has been approved by nikomatsakis

@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 Apr 7, 2020
@Dylan-DPC-zz
Copy link

@bors p=1

@bors
Copy link
Contributor

bors commented Apr 8, 2020

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 8, 2020
@lcnr lcnr force-pushed the has_local_value branch from 3358803 to fca7d16 Compare April 8, 2020 21:34
@lcnr
Copy link
Contributor Author

lcnr commented Apr 8, 2020

should once again be ready

@varkor
Copy link
Member

varkor commented Apr 8, 2020

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Apr 8, 2020

📌 Commit fca7d16 has been approved by nikomatsakis

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 8, 2020
@bors
Copy link
Contributor

bors commented Apr 9, 2020

⌛ Testing commit fca7d16 with merge 11f6096...

@bors
Copy link
Contributor

bors commented Apr 9, 2020

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing 11f6096 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 9, 2020
@bors bors merged commit 11f6096 into rust-lang:master Apr 9, 2020
@bors bors mentioned this pull request Apr 9, 2020
Merged
@lcnr lcnr deleted the has_local_value branch April 9, 2020 06:56
@jyn514 jyn514 mentioned this pull request Nov 1, 2020
Closed
@SNCPlay42 SNCPlay42 mentioned this pull request Apr 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eddyb eddyb eddyb approved these changes

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace calls to has_local_value with needs_infer.
7 participants
@lcnr @eddyb @nikomatsakis @bors @Dylan-DPC-zz @varkor @rust-highfive