Stream: t-compiler

Topic: #54883 -- or-patterns


centril (Oct 18 2019 at 12:57, on Zulip):

@dlrobertson @varkor @Matthew Jasper :wave: -- I thought I'd make a Zulip thread for the work

dlrobertson (Oct 18 2019 at 13:15, on Zulip):

Awesome

dlrobertson (Oct 18 2019 at 13:15, on Zulip):

I'm a little stuck on generating the correct slice or vector of Candidates

dlrobertson (Oct 18 2019 at 13:17, on Zulip):

The problem boils down to the fact that the current algorithm assumes the number of patterns/match pairs is not growable, but we're obviously changing that

dlrobertson (Oct 18 2019 at 13:27, on Zulip):

The case <pat>(<pat> | <pat>) is super simple and is very easy to get working

dlrobertson (Oct 18 2019 at 13:28, on Zulip):

But when you have multiple match pairs that are or-patterns that's when things get way more complicated

dlrobertson (Oct 18 2019 at 13:29, on Zulip):

In every implementation I've tried so far I either don't generate the correct test or don't clear the candidates match pairs correctly, so I hit an assertion later

dlrobertson (Oct 18 2019 at 13:30, on Zulip):

Thoughts and feedback would be super helpful

centril (Oct 18 2019 at 13:30, on Zulip):

@dlrobertson Is there perhaps a more targeted PR that we can land first for the easy subset that doesn't create roadblocks for the more advanced case?

centril (Oct 18 2019 at 13:30, on Zulip):

Might be good purely for testing purposes

dlrobertson (Oct 18 2019 at 13:30, on Zulip):

Maybe the single or pattern?

centril (Oct 18 2019 at 13:31, on Zulip):

Yeah that's what I'm thinking

dlrobertson (Oct 18 2019 at 13:31, on Zulip):

E.g. Foo(1 | 2, 3 | 4) throws a bug

centril (Oct 18 2019 at 13:32, on Zulip):

:+1:

dlrobertson (Oct 18 2019 at 13:33, on Zulip):

I can post an update to the PR that implements this in a Jacky manner and then maybe @varkor or @Matthew Jasper will spot the reason why multiple or patterns fail

centril (Oct 18 2019 at 13:34, on Zulip):

@dlrobertson you could also file a smaller extraction-PR for the subset

centril (Oct 18 2019 at 13:34, on Zulip):

(I find that easier so I don't lose my original work)

centril (Oct 18 2019 at 13:36, on Zulip):

cc also @nikomatsakis, @pnkfelix

dlrobertson (Oct 27 2019 at 22:59, on Zulip):

@centril been working on an update today... I'll post a PR shortly

centril (Oct 27 2019 at 22:59, on Zulip):

Nice!

dlrobertson (Oct 27 2019 at 22:59, on Zulip):

Most everything is passing locally at the moment

dlrobertson (Oct 27 2019 at 23:00, on Zulip):

I'm sure there are holes in it and it is super hacky

centril (Oct 27 2019 at 23:00, on Zulip):

Glad to see progress nonetheless; soon we'll be able to announce to the world that they should try the implementation out

dlrobertson (Oct 27 2019 at 23:02, on Zulip):

as long as all bugs are attributed to work you did :smiley:

centril (Oct 27 2019 at 23:02, on Zulip):

=P

dlrobertson (Oct 27 2019 at 23:02, on Zulip):

jk if anything the errors will surface in the lowering

dlrobertson (Oct 27 2019 at 23:03, on Zulip):

I was curious how rustfmt works with the or-pattern?

centril (Oct 27 2019 at 23:03, on Zulip):

I'd be very happy if bugs are found and reported; test cases for free! :tada:

centril (Oct 27 2019 at 23:03, on Zulip):

@dlrobertson no idea if it is implemented or not

centril (Oct 27 2019 at 23:03, on Zulip):

but it should probably be since we made AST level changes

dlrobertson (Oct 27 2019 at 23:04, on Zulip):

I know they have special rules for | at the end vs at the beginning

dlrobertson (Oct 27 2019 at 23:04, on Zulip):

yeah I'd think it would _just work_ but

dlrobertson (Oct 27 2019 at 23:04, on Zulip):

was curious

centril (Oct 27 2019 at 23:06, on Zulip):

Seems like it is implemented:

fn foo() {
    match 0 {
        Some(
            VeryLongThing
            | JesusHowLongThisIs
            | IMeanHaveYouSeenSomethingSoLong
            | AndItGetsEvenLonger,
        ) => {}
    }
}
centril (Oct 27 2019 at 23:07, on Zulip):

@dlrobertson looks reasonable

dlrobertson (Oct 27 2019 at 23:07, on Zulip):

:tada: nice!

dlrobertson (Oct 28 2019 at 00:28, on Zulip):

:/ need to do a rebase on upstream... will be a sec to rebuild and test

dlrobertson (Oct 28 2019 at 00:28, on Zulip):

if I don't push the changes up tonight, I'll push them tomorrow

dlrobertson (Oct 28 2019 at 02:45, on Zulip):

pushed the changes... its definitely still in the phase where it is largely a hack and less of a feature

dlrobertson (Oct 28 2019 at 02:45, on Zulip):

did quite a bit of testing and a lot seems to work out of the box

dlrobertson (Oct 28 2019 at 02:46, on Zulip):

largely due to the move from manually building the CFG -> using the Candidate

dlrobertson (Oct 28 2019 at 02:48, on Zulip):

but @ currently causes an ICE and unreachability checks trigger some false warnings

dlrobertson (Oct 28 2019 at 02:48, on Zulip):

I'm sure I'll find more as I go

centril (Oct 28 2019 at 02:50, on Zulip):

Hopefully @Matthew Jasper will pitch in at some point :slight_smile:

Matthew Jasper (Oct 28 2019 at 20:21, on Zulip):

I'll have a look at this at the weekend. I'll need to experiment a bit to work out what the final version of the code should look like.

Matthew Jasper (Nov 02 2019 at 22:46, on Zulip):

@dlrobertson I've pulled your branch and experimented with it locally a bit. There seem to be some ICEs and unexpected errors from match checking which I don't know how to fix, I guess @Nadrieril will be fixing them.

dlrobertson (Nov 02 2019 at 22:52, on Zulip):

Depends on what the ICE is I guess?

dlrobertson (Nov 02 2019 at 22:53, on Zulip):

I did hit ICEs when using @

dlrobertson (Nov 02 2019 at 22:53, on Zulip):

What were you hitting an ICE with?

Matthew Jasper (Nov 02 2019 at 22:54, on Zulip):

This assert is firing:

build/matches/mod.rs:1215 |    assert!(rows.iter().all(|r| r.len() == v.len()));

test code:

pub fn f() -> i32 {
    let x = ((1,),);

    match x {
        ((y,) | (y,),) => 2,
        _ => 4,
    }
}
Matthew Jasper (Nov 02 2019 at 22:56, on Zulip):

I've left a comment trying to explain what I was expecting the lowering code to look like.

Last update: Nov 16 2019 at 01:05UTC