Stream: t-compiler/wg-prioritization/alerts

Topic: I-prioritize #77218 ICE on `if let Some() = ...` expression…


triagebot (Sep 26 2020 at 13:03, on Zulip):

@WG-prioritization/alerts issue #77218 has been requested for prioritization.

Procedure

lcnr (Sep 26 2020 at 13:06, on Zulip):

Looks P-high to me, to me it seems like there could be something serious going wrong here

Stu (Sep 26 2020 at 13:06, on Zulip):

Agree. It's also a regression from stable to nightly

LeSeulArtichaut (Sep 26 2020 at 13:17, on Zulip):

The code shouldn’t compile though, right?

Stu (Sep 26 2020 at 13:17, on Zulip):

No it shouldn't, since the let is missing.
If you compile on stable, it will report a normal error

LeSeulArtichaut (Sep 26 2020 at 13:18, on Zulip):

And on nightly there’s no error before the ICE?

Stu (Sep 26 2020 at 13:18, on Zulip):

no

LeSeulArtichaut (Sep 26 2020 at 13:18, on Zulip):

Right

LeSeulArtichaut (Sep 26 2020 at 13:19, on Zulip):

+1 for P-high as a fairly bad diagnostic regression

LeSeulArtichaut (Sep 26 2020 at 13:20, on Zulip):

I wonder if we could minimize the example further

Stu (Sep 26 2020 at 13:21, on Zulip):

I tried to, but I think it has something to do with the lifetimes because my code failed normally without the lifetimes and structs

lcnr (Sep 26 2020 at 13:21, on Zulip):

Stu said:

I tried to, but I think it has something to do with the lifetimes because my code failed normally without the lifetimes and structs

hmm, let me look at this again in this case

LeSeulArtichaut (Sep 26 2020 at 13:21, on Zulip):

Sounds plausible

Stu (Sep 26 2020 at 13:22, on Zulip):

I haven't really tried to minimize it, so go ahead and try yourself

LeSeulArtichaut (Sep 26 2020 at 13:22, on Zulip):

@lcnr Do you think you know the culprit? Or should we go ahead and ping cleanup for bisection?

lcnr (Sep 26 2020 at 13:32, on Zulip):

I have a guess, but still think bisection is useful here

James Gill (Sep 26 2020 at 13:32, on Zulip):

LeSeulArtichaut said:

lcnr Do you think you know the culprit? Or should we go ahead and ping cleanup for bisection?

Currently running a bisection and am getting used to the tool as I am new to it. Would this case need any more config than running it raw against the code produced in this issue - due to if you build on stable it'll error out normally?

Stu (Sep 26 2020 at 13:32, on Zulip):

I bisected it

Stu (Sep 26 2020 at 13:33, on Zulip):

@James Gill you can use the --prompt parameter to do it interactively. If the ICE happens, Mark as regressed, if normal error, Mark as baseline

James Gill (Sep 26 2020 at 13:34, on Zulip):

Ahh cool, thanks!

Stu (Sep 26 2020 at 13:44, on Zulip):

Ah you can also provide --regress=ice flag to check for an ICE

Nelson J Morais (Sep 26 2020 at 13:45, on Zulip):

would make sense to think about the possibility to add an extra optional flag to --prompt like a txt file path where its content is a substring of the compilation output. basicly telling bisct-rustc: when this substring is not found in the output then stop and lemme choose regression/baseline/retry else assume X and keep going ? (were X is regressed or baseline)

Nelson J Morais (Sep 26 2020 at 13:46, on Zulip):

i'm also new here and it's time consuming when you do an --end 2020-09-26 --prompt. i still didn't got to my baseline yet :sweat_smile:

Nelson J Morais (Sep 26 2020 at 13:48, on Zulip):

@Stu ah looks like --regress=ice will do more or less as i'm sugesting right?

Stu (Sep 26 2020 at 13:48, on Zulip):

Yes

LeSeulArtichaut (Sep 26 2020 at 15:44, on Zulip):

Also with cargo-bisect-rustc you can set a shell script to do the check for you

LeSeulArtichaut (Sep 26 2020 at 15:44, on Zulip):

Given that ICEs have a different return code than "normal" failures, you can automate the process fairly easily

LeSeulArtichaut (Sep 26 2020 at 15:46, on Zulip):

Btw there's a #t-compiler/cargo-bisect-rustc stream here also

Camelid (Sep 26 2020 at 17:16, on Zulip):

LeSeulArtichaut said:

I wonder if we could minimize the example further

Minimized to:

pub struct Cache {
    cached_data_refs: Vec<i32>,
}

impl Cache {
    pub fn data_ref_list(&mut self, key: &usize) {
        for reference in vec![1, 2, 3] {
            // Following `if` statement is the reason for compiler crash. To trigger the crash it
            // is required to:
            //
            // * have `let` keyword missing from `if let Some(...)` expression
            // * have binding name in the `Some(...)` portion of the `if let Some()` shadow the
            //   name from the outer loop
            //
            if /*let*/ Some(reference) = self.cached_data_refs.get(*key) {
                unimplemented!()
            }
        }
    }
}
triagebot (Sep 26 2020 at 17:18, on Zulip):

Issue #77218's prioritization request has been removed.

LeSeulArtichaut (Oct 03 2020 at 09:49, on Zulip):

I guess it would be nice if #77283 could get merged before master is promoted to beta (on Tuesday). Should we ping potential reviewers?

DPC (Oct 03 2020 at 12:27, on Zulip):

from my understanding of that PR, it doesn't fix the ICE just prevents people from using that code that may ICE

LeSeulArtichaut (Oct 03 2020 at 12:31, on Zulip):

Well there’s "to not ICE" in the PR title x)

Camelid (Oct 03 2020 at 18:09, on Zulip):

LeSeulArtichaut said:

I guess it would be nice if #77283 could get merged before master is promoted to beta (on Tuesday). Should we ping potential reviewers?

I think that would probably be good. Here are the other members of wg-diagnostics:

LeSeulArtichaut (Oct 03 2020 at 18:11, on Zulip):

Those are Yuki Okushi davidtwco and oli on Zulip, unfortunately they don't seem to be available right now

LeSeulArtichaut (Oct 03 2020 at 18:12, on Zulip):

The code touches typeck though, we might want to ask typecheck experts instead

LeSeulArtichaut (Oct 03 2020 at 18:14, on Zulip):

But anyway I think backporting is perfectly feasible if it's not merged in time

DPC (Oct 03 2020 at 20:19, on Zulip):

yeah let's not hurry on a review now, it's fine if this doesn't get merged when beta is cut

Last update: Apr 15 2021 at 03:15UTC