Stream: t-compiler

Topic: nested impl trait warning period PR #58608


pnkfelix (Mar 11 2019 at 11:34, on Zulip):

hey @Zoxc can you clarify why you don't think we can get away with just setting the warning_period_57979_nested_impl_trait back to false at the "right" point(s) in the control flow?

pnkfelix (Mar 11 2019 at 11:35, on Zulip):

Its not that I'm trying to argue that you are wrong; if anything, the failure of my prior attempt to do this makes me think that you are right

pnkfelix (Mar 11 2019 at 11:35, on Zulip):

but I nonetheless haven't grokked why the old NestedImplTraitVisitor is necessary to exhibit the behavior

Zoxc (Mar 11 2019 at 11:37, on Zulip):

It seems hard to reason about and it's possible fragile since the AST validator is likely to be changed. I'm not saying it's impossible, but it's probably not worthwhile.

pnkfelix (Mar 11 2019 at 11:39, on Zulip):

yeah, the "worthwhile use of programming effort" point is definitely the one I'm struggling with

Zoxc (Mar 11 2019 at 11:39, on Zulip):

Given that we're going to remove this code later, I'd go with the easy option

pnkfelix (Mar 11 2019 at 12:44, on Zulip):

Ah! now I see where the original logic truly went wrong. I had thought the issue was that skipping visit_ty meant that it missed the check for an outer impl trait; but that wasn't the problem at all. It was that calling walk_ty instead of visit_ty meant that we missed a single overwrite of an outer impl trait.

pnkfelix (Mar 11 2019 at 12:44, on Zulip):

this misunderstanding of my own explains why I had such difficulty making test cases that would illustrate the fine distinctions here.

pnkfelix (Mar 11 2019 at 15:32, on Zulip):

okay I've updated the PR now. I think this captures the previous behavior very precisely.

pnkfelix (Mar 12 2019 at 11:53, on Zulip):

hey, @Zoxc , you said your feedback about the incorrect comment was not addressed, but I thought I did address it

pnkfelix (Mar 12 2019 at 11:53, on Zulip):

are you talking about this feedback ?

pnkfelix (Mar 12 2019 at 11:54, on Zulip):

Oh maybe I misunderstood your comment

pnkfelix (Mar 12 2019 at 11:55, on Zulip):

I thought you were objecting to the use of the word "incomplete" (and were suggesting I say "bugged" instead, and I settled on "buggy")

pnkfelix (Mar 12 2019 at 11:55, on Zulip):

but re-reading interaction now, I realize that you may have been objecting to my phrasing "until sometime after PR #57730 landed", and that you'd prefer "until PR #57730 landed"

Zoxc (Mar 12 2019 at 11:56, on Zulip):

Yeah

pnkfelix (Mar 12 2019 at 11:56, on Zulip):

the reason I chose the phrasing "sometime after #57730" is because the analysis even with #57730 was still buggy.

pnkfelix (Mar 12 2019 at 11:57, on Zulip):

I could say "until PRs #57730 and #57981 landed"

pnkfelix (Mar 12 2019 at 11:57, on Zulip):

would that be satisfactory?

Zoxc (Mar 12 2019 at 11:57, on Zulip):

Sure

pnkfelix (Mar 12 2019 at 11:57, on Zulip):

okay great. I'll make that change and then @bors r=zoxc. Thanks!

Last update: Nov 22 2019 at 05:00UTC