[clang] Set the error-bit for ill-formed semantic InitListExpr.

[hokein]

Summary

When a semantic checking fails on a syntactic InitListExpr, we will
get an ill-formed semantic InitListExpr (e.g. some inits are nullptr),
using this semantic InitListExpr in clang (without setting the err-bits) is crashy.

[sammccall]

clang/lib/Sema/SemaInit.cpp

965

Can we have a clang test for this? If nothing else, an AST-dump test should be able to capture this, right?

If we can turn it into a real clang crash, we may be able to get this on the release branch.

[sammccall]

clang/include/clang/AST/Expr.h

4697

Hmm, actually hardcoding the error->VI relationship seems like a smell here...
I'm not sure what the best fix is though, any ideas?

[hokein]

clang/include/clang/AST/Expr.h

4697

yeah, this is duplicated with the one in computeDependence(RecoveryExpr).

ideas:

1. making this markError public method in Expr and use it computeDependence, so that we have a single place.
2. add an alias ErrorDependent = Error | ValueInstantiation in the enum ExprDependence

any preference? personally, I'd prefer 2.

clang/lib/Sema/SemaInit.cpp

965

I tried it, but no luck to get such testcase :(

ast-dump doesn't really help, it just dumps the syntactic form, not the semantic form.

another option will be to write an unittest in AST, implementing a RAV to retrieve the semantic form of the ini-list-expr, and check the error-bit.

[hokein]

add "ErrorDependent" alias to ExprDependence enum.

clang-tools-extra/clangd/unittests/HoverTests.cpp

llvm::StringRef Tests[] = {
           )cpp",
       R"cpp(// non-named decls don't get hover. Don't crash!
             ^static_assert(1, "");
           )cpp",
       R"cpp(// non-evaluatable expr
           template <typename T> void foo() {
             (void)[[size^of]](T);
           })cpp",
+      R"cpp(// should not crash on invalid semantic form of init-list-expr.
+            /*error-ok*/
+            struct Foo {
+              int xyz = 0;
+            };
+            class Bar {};
+            constexpr Foo s = ^{
+              .xyz = Bar(),
+            };
+          )cpp",
       // literals
       "auto x = t^rue;",
       "auto x = '^A';",
       "auto x = ^(int){42};",
       "auto x = ^42.;",
       "auto x = ^42.0i;",
       "auto x = ^42;",
       "auto x = ^nullptr;",

clang/include/clang/AST/DependenceFlags.h

enum ExprDependence : uint8_t {
 
     None = 0,
     All = 31,
 
     TypeValue = Type | Value,
     TypeInstantiation = Type | Instantiation,
     ValueInstantiation = Value | Instantiation,
     TypeValueInstantiation = Type | Value | Instantiation,
+    ErrorDependent = Error | ValueInstantiation,
 
     LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Error)
   };
 };
 using ExprDependence = ExprDependenceScope::ExprDependence;
 
 struct TypeDependenceScope {
   enum TypeDependence : uint8_t {

clang/include/clang/AST/Expr.h

public:
   void setInit(unsigned Init, Expr *expr) {
     assert(Init < getNumInits() && "Initializer access out of range!");
     InitExprs[Init] = expr;
 
     if (expr)
       setDependence(getDependence() | expr->getDependence());
   }
 
+  /// Mark the semantic form of the InitListExpr as error when the semantic
+  /// analysis fails.
+  void markError() {
+    assert(isSemanticForm());
+    setDependence(getDependence() | ExprDependence::ErrorDependent);
+  }
+
   /// Reserve space for some number of initializers.
   void reserveInits(const ASTContext &C, unsigned NumInits);
 
   /// Specify the number of initializers
   ///
   /// If there are more than @p NumInits initializers, the remaining
   /// initializers will be destroyed. If there are fewer than @p
   /// NumInits initializers, NULL expressions will be added for the

clang/lib/AST/ComputeDependence.cpp

 ExprDependence clang::computeDependence(RecoveryExpr *E) {
   // RecoveryExpr is
   //   - always value-dependent, and therefore instantiation dependent
   //   - contains errors (ExprDependence::Error), by definition
   //   - type-dependent if we don't know the type (fallback to an opaque
   //     dependent type), or the type is known and dependent, or it has
   //     type-dependent subexpressions.
   auto D = toExprDependence(E->getType()->getDependence()) |
-           ExprDependence::ValueInstantiation | ExprDependence::Error;
+           ExprDependence::ErrorDependent;
   // FIXME: remove the type-dependent bit from subexpressions, if the
   // RecoveryExpr has a non-dependent type.
   for (auto *S : E->subExpressions())
     D |= S->getDependence();
   return D;
 }
 
 ExprDependence clang::computeDependence(PredefinedExpr *E) {

clang/lib/Sema/SemaInit.cpp

InitListChecker::InitListChecker(Sema &S, const InitializedEntity &Entity,
   if (!hadError && FullyStructuredList) {
     bool RequiresSecondPass = false;
     FillInEmptyInitializations(Entity, FullyStructuredList, RequiresSecondPass,
                                /*OuterILE=*/nullptr, /*OuterIndex=*/0);
     if (RequiresSecondPass && !hadError)
       FillInEmptyInitializations(Entity, FullyStructuredList,
                                  RequiresSecondPass, nullptr, 0);
   }
+  if (hadError && FullyStructuredList)
+    FullyStructuredList->markError();
 }
 
 int InitListChecker::numArrayElements(QualType DeclType) {
   // FIXME: use a proper constant
   int maxElements = 0x7FFFFFFF;
   if (const ConstantArrayType *CAT =
         SemaRef.Context.getAsConstantArrayType(DeclType)) {
     maxElements = static_cast<int>(CAT->getSize().getZExtValue());