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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3459-3461
+      if (MemInit.get()->getInit() &&
+          MemInit.get()->getInit()->containsErrors())
+        AnyErrors = true;
----------------
The parser should not be inspecting properties of `Expr`s -- that's a layering violation. Can you move this logic into `ActOnMemInitializers` instead?


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1688
                                             CheckConstexprKind Kind) {
+  assert(!NewFD->isInvalidDecl());
   const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(NewFD);
----------------
Instead of asserting this, please just return false on an invalid declaration. There's nothing fundamentally wrong with calling this function on an invalid declaration, but as usual, if we encounter something invalid, we should suppress follow-on diagnostics.


================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5005
+    if (Member->getInit() && Member->getInit()->containsErrors())
+      Constructor->setInvalidDecl();
     if (Member->isBaseInitializer())
----------------
This is inappropriate. The "invalid" bit on a declaration indicates whether the declaration is invalid, not whether the definition is invalid.

What we should do is add a "contains errors" bit to `Stmt`, and mark the function body as containing errors if it has an initializer that contains an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77041