[PATCH] D113752: [Parse] Use empty RecoveryExpr when if/while/do/switch conditions fail to parse

Thu Dec 30 06:14:03 PST 2021

sammccall marked 6 inline comments as done.
sammccall added a comment.

In D113752#3188486 <https://reviews.llvm.org/D113752#3188486>, @hokein wrote:

> since we're now preserving  more invalid code, we should check whether const-evaluator is cable of handling these newly-added invalid case.
>
> - the follow case will result an value-dependent violation, the fix would be to handle value-dependent condition in `EvaluateSwitch` (`clang/lib/AST/ExprConstant.cpp`)

Great catch, thanks!
I added constant-evaluation to ensure we don't crash for all the varieties of broken loops I could think of.
They're in `SemaCXX/constexpr-function-recovery-crash.cpp` which seems to be very similar. Maybe slightly misplaced as really we're testing AST/ExprConstant.

================
Comment at: clang/test/Parser/cxx0x-attributes.cpp:155
       alignas(4 ns::i; // expected-note {{to match this '('}}
+                       // expected-error at -1 {{expected ';' after do/while}}
 } // expected-error 2{{expected ')'}} expected-error {{expected expression}}
----------------
hokein wrote:
> This looks like a bogus diagnostic, but I think it is ok, as this is a poor-recovery case for clang already -- IIUC, the do-while statement range claims to the `}` on Line56.
> 
> this is a case which can be improved by our bracket-matching repair :)
Right. My understanding is the diagnostic is bogus if you put both missing `)`s before the semicolon, but clang is noticing them/implicitly inserting them before the `}`.

The code is horribly mangled here in any case, I'm not sure what diagnostic I'd want as a user really.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113752/new/

https://reviews.llvm.org/D113752