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

[Haojian Wu via Phabricator via cfe-commits]

hokein marked 2 inline comments as done.
hokein added inline comments.


================
Comment at: clang/include/clang/AST/Expr.h:4697
+    assert(isSemanticForm());
+    setDependence(getDependence() | ExprDependence::Error |
+                  ExprDependence::ValueInstantiation);
----------------
sammccall wrote:
> Hmm, actually hardcoding the error->VI relationship seems like a smell here...
> I'm not sure what the best fix is though, any ideas?
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.


================
Comment at: clang/lib/Sema/SemaInit.cpp:965
   }
+  if (hadError && FullyStructuredList)
+    FullyStructuredList->markError();
----------------
sammccall wrote:
> 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.
> 
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84140