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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
