Skip to content

Introduce ReError #107652

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

Merged
merged 6 commits into from
Feb 10, 2023
Merged

Introduce ReError #107652

merged 6 commits into from
Feb 10, 2023

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Feb 3, 2023

CC #69314

r? @nagisa

@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 Feb 3, 2023
Comment on lines +12 to +21
error[E0597]: `buf` does not live long enough
--> $DIR/issue-69314.rs:14:19
|
LL | let m2 = &buf[..];
| ^^^ borrowed value does not live long enough
LL | let m = Self::g(m2).await;
| ----------- argument requires that `buf` is borrowed for `'static`
LL | Self::f2(m).await;
LL | }
| - `buf` dropped here while still borrowed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to hide this error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've looked at this and it seems that Self::f2 doesn't seem to get around to influencing the lifetime requirements from Self::g, let's follow up on this in a subsequent PR.

@compiler-errors
Copy link
Member

compiler-errors commented Feb 3, 2023

cc @rust-lang/types who might have opinions about this approach.

I think making ReError should require delaying a bug or providing a ErrorGuaranteed like the error ty kind, and it should store it in the variant (ErrorGuaranteed is a zst iirc, so it shouldn't have any overhead). Then we can just ignore it during type relations, instead of delaying a bug there.

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

we may want to make FlagComputation::add_region also set the error flag for error regions, but that may cause additional diagnostics changes, so maybe do it in a follow up PR

Comment on lines 1043 to 1047
VarValue::ErrorValue => tcx.lifetimes.re_static,
VarValue::ErrorValue => tcx.re_error(),
Copy link
Contributor

Choose a reason for hiding this comment

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

we may want to carry an ErrorGuaranteed through ErrorValue in the future, doesn't have to be this PR tho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I started doing this and realized that where we construct the ErrorValue we don't have easy access to ErrorGuaranteed, so I'll leave this as follow up work, as suggested.

@oli-obk
Copy link
Contributor

oli-obk commented Feb 8, 2023

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned nagisa Feb 8, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Feb 9, 2023

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 9, 2023

📌 Commit 3689295 has been approved by oli-obk

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 Feb 9, 2023
@bors
Copy link
Collaborator

bors commented Feb 10, 2023

⌛ Testing commit 3689295 with merge d1ac43a...

@bors
Copy link
Collaborator

bors commented Feb 10, 2023

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing d1ac43a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 10, 2023
@bors bors merged commit d1ac43a into rust-lang:master Feb 10, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 10, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d1ac43a): 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)
4.3% [4.3%, 4.3%] 1
Regressions ❌
(secondary)
3.1% [2.4%, 3.8%] 12
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 4.3% [4.3%, 4.3%] 1

Cycles

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)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 3.1% [3.1%, 3.1%] 1

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.

7 participants