[PATCH] D77041: [AST] Fix a crash on invalid constexpr Ctorinitializer when building RecoveryExpr.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

> This makes me nervous, marking the constructor as invalid seems much safer. Can you show tests it regresses?

Yeah, the first version of the patch doesn't seem to fix all crashes (X().Y would lead another crash)...

so if we mark the CtorDecl as invalid

1. whenever `CtorInitializer->init()->containsError`, we don't have failure tests
2. whenever there is any kind of errors (including the containsError case above) in CtorInitailizer, we have three failing tests (SemaCXX/constant-expression-cxx11.cpp, SemaCXX/constructor-initializer.cpp, SemaTemplate/constexpr-instantiate.cpp).

though 1) passes all existing tests, I think it just means current tests don't have enough coverage for recoveryExpr cases. But given the current state, 1) looks most promising -- fixes the crashes, retains broken expressions in CtorInitializer rather than dropping them, doesn't regress the diagnostics a lot (only for CtorInitializer->init()->containsError` case).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77041