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?
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
but I nonetheless haven't grokked why the old NestedImplTraitVisitor is necessary to exhibit the behavior
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.
yeah, the "worthwhile use of programming effort" point is definitely the one I'm struggling with
Given that we're going to remove this code later, I'd go with the easy option
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.
this misunderstanding of my own explains why I had such difficulty making test cases that would illustrate the fine distinctions here.
okay I've updated the PR now. I think this captures the previous behavior very precisely.
hey, @Zoxc , you said your feedback about the incorrect comment was not addressed, but I thought I did address it
are you talking about this feedback ?
Oh maybe I misunderstood your comment
I thought you were objecting to the use of the word "incomplete" (and were suggesting I say "bugged" instead, and I settled on "buggy")
would that be satisfactory?
okay great. I'll make that change and then
@bors r=zoxc. Thanks!