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.

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

Don't check for alias bounds in liveness when aliases have escaping bound vars #117466

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

compiler-errors
Copy link
Member

I actually have no idea how we should be treating aliases with escaping bound vars here... but the simplest behavior is just doing what we used to do before.

r? aliemjay

Fixes #117455

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 31, 2023
@tmandry
Copy link
Member

tmandry commented Nov 1, 2023

Friendly ping, this is breaking our build and I'd rather see a fix forward land soon.

If it helps: I haven't seen the code where this visitor is used, but the diff looks reasonable and the strategy of "do what we did before" sounds safe. If you're confident it's not horribly broken you can r=me.

@aliemjay
Copy link
Member

aliemjay commented Nov 2, 2023

I have thought about "smarter" solutions but they all turned to be incorrect (unsound?) because of the way we handle well-formedness around higher ranked types. Now I'm fairly convinced that the approach of this PR is the only way handle escaping bound vars.

Accordingly I'd prefer removing the FIXME comment and adding the following test to make sure we are not trying to do anthing smarter in this inference but you can r=me without these changes.

trait Trait {
    type Gat<'a: 'b, 'b: 'c, 'c>: 'c;
}

fn get_func<'a, T: Trait>(_: &'a str) -> fn(T::Gat<'a, '_, 'static>) {
    loop {}
}

fn test<T: Trait>() {
    let func = get_func::<T>(&String::new()); //~ ERROR temporary value dropped
    drop(func);
}

MentionsLifetimeAndType(&(), foo);
}

fn main() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

this test doesn't contain an opaque. Please add a comment referencing #117455 and one or two sentences explaining what this is testing

@compiler-errors
Copy link
Member Author

Added aliemjay's test, added a comment and moved the original test cause it didn't have a TAIT anyways.

@bors r=aliemjay

@bors
Copy link
Contributor

bors commented Nov 2, 2023

📌 Commit 4d5d763 has been approved by aliemjay

It is now in the queue for this repository.

@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 Nov 2, 2023
@bors
Copy link
Contributor

bors commented Nov 2, 2023

⌛ Testing commit 4d5d763 with merge b800c30...

@bors
Copy link
Contributor

bors commented Nov 2, 2023

☀️ Test successful - checks-actions
Approved by: aliemjay
Pushing b800c30 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 2, 2023
@bors bors merged commit b800c30 into rust-lang:master Nov 2, 2023
11 checks passed
@rustbot rustbot added this to the 1.75.0 milestone Nov 2, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b800c30): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.9% [-2.9%, -2.9%] 1
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 639.068s -> 636.285s (-0.44%)
Artifact size: 304.47 MiB -> 304.50 MiB (0.01%)

bors-ferrocene bot added a commit to ferrocene/ferrocene that referenced this pull request Nov 3, 2023
82: Automated pull from upstream `master` r=tshepang a=github-actions[bot]


This PR pulls the following changes from the upstream repository:

* rust-lang/rust#117313
* rust-lang/rust#117131
* rust-lang/rust#117134
* rust-lang/rust#117471
* rust-lang/rust#117521
* rust-lang/rust#117513
  * rust-lang/rust#117512
  * rust-lang/rust#117509
  * rust-lang/rust#117495
  * rust-lang/rust#117394
* rust-lang/rust#117466
* rust-lang/rust#117204
* rust-lang/rust#117386
* rust-lang/rust#117506



Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com>
Co-authored-by: roblabla <unfiltered@roblab.la>
Co-authored-by: Michael Goulet <michael@errs.io>
Co-authored-by: massivebird <gdrakemail@gmail.com>
Co-authored-by: bors <bors@rust-lang.org>
Co-authored-by: Zalathar <Zalathar@users.noreply.github.com>
Co-authored-by: lcnr <rust@lcnr.de>
Co-authored-by: Joshua Liebow-Feeser <joshlf@users.noreply.github.com>
Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
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.

ICE: assertion failed: !verify_if_eq_b.has_escaping_bound_vars()
7 participants