Stream: t-compiler/wg-polonius

Topic: hybrid algorithm in rustc


Albin Stjerna (Mar 15 2019 at 10:44, on Zulip):

Ok, so how does the versioning for polonius-engine work; the one with the Hybrid engine becomes 0.6.3? Also, should I do the version increment as part of the pull request or is there some other standard way of doing that? I realise I am asking for the mentoring instructions task early :)

Albin Stjerna (Mar 15 2019 at 10:45, on Zulip):

Also, is there any particular reason why we aren't using Criterion for benchmarking? It seems incredibly cool to me, and I really like the repeated benchmarks to ensure that the changes are statistically significant

Albin Stjerna (Mar 15 2019 at 10:48, on Zulip):

Can I even Cargo publish new versions? I assume one needs some sort of special powers for that?

nikomatsakis (Mar 15 2019 at 13:10, on Zulip):

Ok, so how does the versioning for polonius-engine work; the one with the Hybrid engine becomes 0.6.3? Also, should I do the version increment as part of the pull request or is there some other standard way of doing that? I realise I am asking for the mentoring instructions task early :)

Yeah, I guess this would be 0.6.3 -- technically, I suppose, extending a public enum is a breaking change though so we could make it 0.70

nikomatsakis (Mar 15 2019 at 13:10, on Zulip):

Also, is there any particular reason why we aren't using Criterion for benchmarking? It seems incredibly cool to me, and I really like the repeated benchmarks to ensure that the changes are statistically significant

not really, except that generated random data that is meaningful to polonius is non-trivial

nikomatsakis (Mar 15 2019 at 13:10, on Zulip):

I'm not super familiar with criterion, so maybe I'm saying something confused

nikomatsakis (Mar 15 2019 at 13:11, on Zulip):

In the datafrog repository I did setup some kind of testing at least that was using quickcheck or some variant thereof

nikomatsakis (Mar 15 2019 at 13:11, on Zulip):

that might be a place we could do benchmarking too

Albin Stjerna (Mar 15 2019 at 15:22, on Zulip):

Ah, but Criterion is for benchmarking, not for generating test input, apparently it replaces the built-in benchmarking and does some sort of statistical analysis on repeat experiments

Albin Stjerna (Mar 15 2019 at 15:23, on Zulip):

I think it also has facilities for comparing changes in performance between versions somehow but I haven't at all looked into that

Jake Goulding (Mar 16 2019 at 15:23, on Zulip):

extending a public enum is a breaking change

Sounds like you could add the hidden non-exhaustive enum variant to prevent another major version bump in the future

Albin Stjerna (Mar 18 2019 at 09:41, on Zulip):

add the hidden non-exhaustive enum variant to prevent another major version bump in the future

I am not sure what this means; do you mean to use two enums?

Albin Stjerna (Mar 18 2019 at 09:47, on Zulip):

Also, does anyone know what the next step is for this? I have made the changes to rustc that @nikomatsakis suggested, but I don't know how to go about version-bumping Polonius (modulo the changes @Jake Goulding suggested).

Jake Goulding (Mar 18 2019 at 13:26, on Zulip):

Something like

enum PoloniusAlgo {
    Slow,
    Fast,
    ReallyFast,

    #[doc(hidden)]
    __NonExhaustive,
}

For more context, see RFC 2008

Jake Goulding (Mar 18 2019 at 13:29, on Zulip):

TL;DR: by hiding one variant, it's hard (but not impossible) to match on all of the variants, forcing consumers to have a catch-all variant. Without that last variant, people could write

match algo {
    Slow => {}
    Fast => {}
    ReallyFast => {}
}

And then adding a new variant would cause their code to fail to compile.

Jake Goulding (Mar 18 2019 at 13:32, on Zulip):

instead, with the hidden variant, they are "forced" to write

match algo {
    Slow => {}
    Fast => {}
    ReallyFast => {}
    _ => {}
}

Then adding a new variant will fall into the last arm

Jake Goulding (Mar 18 2019 at 13:33, on Zulip):

There's reasons that both are good — maybe you do want to force people to address the new variant when its added

Jake Goulding (Mar 18 2019 at 13:34, on Zulip):

And bumping a semver-incompatible versions isn't really a big inconvenience.

Albin Stjerna (Mar 18 2019 at 13:36, on Zulip):

Ah, ok. I didn't know about hidden variants, thanks for explaining!

nikomatsakis (Mar 19 2019 at 14:44, on Zulip):

@Albin Stjerna ok I've merged polonius#102 (your PR). The next steps for integrating into rustc would be the following:

1. First, we have to release a new version of polonius-engine
- typically, I do this by creating a PR that bumps the version = "XXX" numbers in the relevant Cargo.toml files -- maybe you want to create this PR?
- then I can run cargo publish
2. Then, we have to open a PR against rustc that changes the Cargo.toml file to use this newer version and updates the source

Albin Stjerna (Mar 19 2019 at 16:03, on Zulip):

1. Done: https://github.com/rust-lang/polonius/pull/103
2. Queued up and waiting for publish!

nikomatsakis (Mar 20 2019 at 19:24, on Zulip):

@Albin Stjerna left a review here

Albin Stjerna (Mar 20 2019 at 19:45, on Zulip):

Albin Stjerna left a review here

Thanks, addressed and pushed!

Last update: Nov 15 2019 at 20:30UTC