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 incorrect delay_span_bug #81532

Merged
merged 1 commit into from
Feb 3, 2021
Merged

Remove incorrect delay_span_bug #81532

merged 1 commit into from
Feb 3, 2021

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jan 29, 2021 •
edited
Loading

The following code is supposed to compile

use std::ops::BitOr;

pub trait IntWrapper {
    type InternalStorage;
}

impl<T> BitOr for dyn IntWrapper<InternalStorage = T>
where
    Self: Sized,
    T: BitOr + BitOr<Output = T>,
{
    type Output = Self;
    fn bitor(self, _other: Self) -> Self {
        todo!()
    }
}

Before this change it would ICE. In #70998 the removed logic was added
to provide better suggestions, and the delay_span_bug guard was added
to protect against a potential logic error when returning traits. As it
happens, there are cases, like the one above, where traits can indeed be
returned, so valid code was being rejected.

Fix (but not close) #80207.

@estebank estebank added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Jan 29, 2021
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(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 Jan 29, 2021
@nagisa
Copy link
Member

nagisa commented Jan 29, 2021

Can the repro be added as a test case?

@Mark-Simulacrum
Copy link
Member

r? @nagisa (or feel free to reassign, but this looks potentially urgent and I don't have the right knowledge to approve quickly I think)

@nagisa
Copy link
Member

nagisa commented Jan 31, 2021

Yeah this seems reasonable enough to me, but this does need a regression test.

@nagisa
Copy link
Member

nagisa commented Jan 31, 2021

Also cc @varkor who r=d the original PR.

@estebank
Remove incorrect delay_span_bug
ede0a71 
The following code is supposed to compile

```rust
use std::ops::BitOr;

pub trait IntWrapper {
    type InternalStorage;
}

impl<T> BitOr for dyn IntWrapper<InternalStorage = T>
where
    Self: Sized,
    T: BitOr + BitOr<Output = T>,
{
    type Output = Self;
    fn bitor(self, _other: Self) -> Self {
        todo!()
    }
}
```

Before this change it would ICE. In rust-lang#70998 the removed logic was added
to provide better suggestions, and the `delay_span_bug` guard was added
to  protect against a potential logic error when returning traits. As it
happens, there are cases, like the one above, where traits can indeed be
returned, so valid code was being rejected.

Fix rust-lang#80207.
@estebank
Copy link
Contributor Author

estebank commented Feb 3, 2021

r? @pnkfelix (just for expedience)

@rust-highfive rust-highfive assigned pnkfelix and unassigned nagisa Feb 3, 2021
@pnkfelix
Copy link
Member

pnkfelix commented Feb 3, 2021

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Feb 3, 2021

📌 Commit ede0a71 has been approved by pnkfelix

@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 3, 2021
@m-ou-se m-ou-se mentioned this pull request Feb 3, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2021
@bors
Auto merge of rust-lang#81718 - m-ou-se:rollup-3ftbymt, r=m-ou-se
120b2a7 
Rollup of 5 pull requests

Successful merges:

 - rust-lang#80394 (make const_err a future incompat lint)
 - rust-lang#81532 (Remove incorrect `delay_span_bug`)
 - rust-lang#81692 (Update clippy)
 - rust-lang#81715 (Reduce tab formatting assertions to debug only)
 - rust-lang#81716 (Fix non-existent-field ICE for generic fields.)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 3, 2021

⌛ Testing commit ede0a71 with merge 120b2a7...

@bors bors merged commit 6695944 into rust-lang:master Feb 3, 2021
@rustbot rustbot added this to the 1.51.0 milestone Feb 3, 2021
@apiraino apiraino added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 4, 2021
@apiraino
Copy link
Contributor

apiraino commented Feb 4, 2021 •
edited
Loading

Beta backport approved, stable backport declined as per team compiler meeting, also mentioned here.

ehuss pushed a commit to ehuss/rust that referenced this pull request Feb 5, 2021
@m-ou-se @ehuss
Rollup merge of rust-lang#81532 - estebank:ice-ice-baby, r=pnkfelix
f6b8e2b 
Remove incorrect `delay_span_bug`

The following code is supposed to compile

```rust
use std::ops::BitOr;

pub trait IntWrapper {
    type InternalStorage;
}

impl<T> BitOr for dyn IntWrapper<InternalStorage = T>
where
    Self: Sized,
    T: BitOr + BitOr<Output = T>,
{
    type Output = Self;
    fn bitor(self, _other: Self) -> Self {
        todo!()
    }
}
```

Before this change it would ICE. In rust-lang#70998 the removed logic was added
to provide better suggestions, and the `delay_span_bug` guard was added
to  protect against a potential logic error when returning traits. As it
happens, there are cases, like the one above, where traits can indeed be
returned, so valid code was being rejected.

Fix (but not close) rust-lang#80207.
@ehuss ehuss mentioned this pull request Feb 5, 2021
Merged
@ehuss ehuss removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 5, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 6, 2021
@bors
Auto merge of rust-lang#81774 - ehuss:beta-backports, r=ehuss
2efd070 
[beta] backports

This backports:

* CI: only copy python.exe to python3.exe if the latter does not exist rust-lang#81762
* Make hitting the recursion limit in projection non-fatal rust-lang#81055
* Remove incorrect `delay_span_bug` rust-lang#81532
* introduce future-compatibility warning for forbidden lint groups rust-lang#81556
* Update cargo rust-lang#81755
* rustdoc: Fix visibility of trait and impl items rust-lang#81288
* Work around missing -dev packages in solaris docker image. rust-lang#81229
* Update LayoutError/LayoutErr stability attributes rust-lang#81767
* Revert 78373 ("dont leak return value after panic in drop") rust-lang#81257
* Rename `panic_fmt` lint to `non_fmt_panic` rust-lang#81729
@apiraino apiraino removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pnkfelix pnkfelix

Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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
Milestone
1.51.0
Development

Successfully merging this pull request may close these issues.
9 participants
@estebank @rust-highfive @nagisa @Mark-Simulacrum @pnkfelix @bors @apiraino @ehuss @rustbot