sammccall added inline comments.
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h:74
bool GCParity;
+ // Have we already used this node for error recovery? (prevents loops)
+ mutable bool Recovered = false;
----------------
hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > haven't look at it deeply -- is this bug related to this eof change? This
> > > looks like a different bug in recovery.
> > They're "related" in that they both fix repeated-recovery scenarios.
> > This change fixes that we can hit an infinite loop when applying recovery
> > repeatedly.
> > The eof change fixes that recovery is (erroneously) only applied once at
> > eof.
> >
> > I hoped to cover them with the same testcase, which tests repeated recovery
> > at EOF. I can extract this change with a separate test if you like, though
> > it will be very similar to the one I have here.
> > This change fixes that we can hit an infinite loop when applying recovery
> > repeatedly.
>
> I'm more worried about this bug, I think this is an important bug, worth a
> separate patch to fix it, right now it looks like a join-effort in the eof
> change.
>
> > The eof change fixes that recovery is (erroneously) only applied once at
> > eof.
>
> Not sure I follow this. I think the eof change is basically to remove a
> technical debt (avoid the special case and repeated code after main parsing
> loop). Am I missing something?
>
> Not sure I follow this. I think the eof change is basically to remove a
> technical debt (avoid the special case and repeated code after main parsing
> loop). Am I missing something?
There's a bug lurking in that tech debt: if recovery does not advance the
cursor then we should go around the loop again, but if it happens at eof then
(in the old code) there's no loop to go around at all.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130550/new/
https://reviews.llvm.org/D130550
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
