Stream: t-compiler/wg-nll

Topic: issue-51345-dead-code-activation


davidtwco (Jul 04 2018 at 19:11, on Zulip):

@nikomatsakis Submitted a PR for this issue. I've not ran all the tests locally but it fixes the repro in the issue.

nikomatsakis (Jul 04 2018 at 19:17, on Zulip):

@David Wood see my review here

davidtwco (Jul 04 2018 at 20:37, on Zulip):

@nikomatsakis Pushed a fix. Didn't take long to do but then spent ages getting confused why the run-pass test was failing even though it was now compiling correctly, then I noticed that the example had panic!()...

nikomatsakis (Jul 04 2018 at 20:38, on Zulip):

heh sorry :)

nikomatsakis (Jul 04 2018 at 20:40, on Zulip):

@David Wood left a few comment nits :)

davidtwco (Jul 04 2018 at 20:42, on Zulip):

@nikomatsakis I'm not sure I follow your first nit.

davidtwco (Jul 04 2018 at 20:43, on Zulip):

Then when we do find an activation we can assert that the field is "not activated".

I took this (from first review) to mean that where we change to ActivatedAt, we should assert that we were at NotActivated (as opposed to NotTwoPhase)?

nikomatsakis (Jul 04 2018 at 20:44, on Zulip):

that is correct

nikomatsakis (Jul 04 2018 at 20:44, on Zulip):

I'm just saying that the purpose of that check

davidtwco (Jul 04 2018 at 20:44, on Zulip):

Are you referring to the comment?

nikomatsakis (Jul 04 2018 at 20:44, on Zulip):

is to make sure that we are "activating" something that we considered 2-phase in the first place

nikomatsakis (Jul 04 2018 at 20:44, on Zulip):

but your comment made it sound like this was checking that we found an activation for each thing

davidtwco (Jul 04 2018 at 20:44, on Zulip):

Ah.

nikomatsakis (Jul 04 2018 at 20:45, on Zulip):

yes, the comment

davidtwco (Jul 04 2018 at 20:45, on Zulip):

I see, I thought you meant the assert! itself, the location or contents of it.

nikomatsakis (Jul 04 2018 at 20:45, on Zulip):

to check that latter thing we'd have to do some kind of iteration at the end

nikomatsakis (Jul 04 2018 at 20:45, on Zulip):

ah, no. the code is fine.

lqd (Jul 04 2018 at 20:45, on Zulip):

"I think this commit" = "this comment"

davidtwco (Jul 04 2018 at 20:45, on Zulip):

I copied the comment from the previous assert that was removed, I'll update it to something more appropriate.

davidtwco (Jul 04 2018 at 20:48, on Zulip):

@nikomatsakis Pushed up the improved comments.

nikomatsakis (Jul 04 2018 at 20:49, on Zulip):

r+, nice

Last update: Nov 22 2019 at 00:40UTC