tmroeder updated this revision to Diff 188307. tmroeder added a comment. Dropped the C++ part of the ImportChooseExpr test entirely.
This is the part that was breaking the tests on Windows, and after further experimentation, it turns out that clang on Windows never expands the template under the conditions of the test. Dropping this test doesn't reduce coverage of the logic regression discussed in the original review. This is explained in the revised summary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58663/new/ https://reviews.llvm.org/D58663 Files: clang/docs/LibASTMatchersReference.html clang/include/clang/ASTMatchers/ASTMatchers.h clang/lib/AST/ASTImporter.cpp clang/lib/ASTMatchers/ASTMatchersInternal.cpp clang/lib/ASTMatchers/Dynamic/Registry.cpp clang/test/ASTMerge/choose-expr/Inputs/choose.c clang/test/ASTMerge/choose-expr/test.c clang/unittests/AST/ASTImporterTest.cpp clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp =================================================================== --- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp +++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp @@ -754,6 +754,11 @@ EXPECT_TRUE(matches("int* i = nullptr;", cxxNullPtrLiteralExpr())); } +TEST(Matcher, ChooseExpr) { + EXPECT_TRUE(matchesC("void f() { (void)__builtin_choose_expr(1, 2, 3); }", + chooseExpr())); +} + TEST(Matcher, GNUNullExpr) { EXPECT_TRUE(matches("int* i = __null;", gnuNullExpr())); } Index: clang/unittests/AST/ASTImporterTest.cpp =================================================================== --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -563,6 +563,17 @@ stringLiteral(hasType(asString("const char [7]")))))); } +TEST_P(ImportExpr, ImportChooseExpr) { + MatchVerifier<Decl> Verifier; + + // This case tests C code that is not condition-dependent and has a true + // condition. + testImport( + "void declToImport() { (void)__builtin_choose_expr(1, 2, 3); }", + Lang_C, "", Lang_C, Verifier, + functionDecl(hasDescendant(chooseExpr()))); +} + TEST_P(ImportExpr, ImportGNUNullExpr) { MatchVerifier<Decl> Verifier; testImport( @@ -1312,6 +1323,27 @@ ASSERT_EQ(ToTemplated1, ToTemplated); } +TEST_P(ASTImporterOptionSpecificTestBase, ImportChooseExpr) { + // This tests the import of isConditionTrue directly to make sure the importer + // gets it right. + Decl *From, *To; + std::tie(From, To) = getImportedDecl( + "void declToImport() { (void)__builtin_choose_expr(1, 0, 1); }", + Lang_C, "", Lang_C); + + auto ToResults = match(chooseExpr().bind("choose"), To->getASTContext()); + auto FromResults = match(chooseExpr().bind("choose"), From->getASTContext()); + + const ChooseExpr *FromChooseExpr = + selectFirst<ChooseExpr>("choose", FromResults); + ASSERT_TRUE(FromChooseExpr); + + const ChooseExpr *ToChooseExpr = selectFirst<ChooseExpr>("choose", ToResults); + ASSERT_TRUE(ToChooseExpr); + + EXPECT_EQ(FromChooseExpr->isConditionTrue(), ToChooseExpr->isConditionTrue()); +} + TEST_P(ASTImporterOptionSpecificTestBase, ImportFunctionWithBackReferringParameter) { Decl *From, *To; Index: clang/test/ASTMerge/choose-expr/test.c =================================================================== --- /dev/null +++ clang/test/ASTMerge/choose-expr/test.c @@ -0,0 +1,4 @@ +// RUN: %clang_cc1 -std=c11 -emit-pch -o %t.ast %S/Inputs/choose.c +// RUN: %clang_cc1 -std=c11 -ast-merge %t.ast -fsyntax-only -verify %s +// expected-no-diagnostics + Index: clang/test/ASTMerge/choose-expr/Inputs/choose.c =================================================================== --- /dev/null +++ clang/test/ASTMerge/choose-expr/Inputs/choose.c @@ -0,0 +1,2 @@ +_Static_assert(__builtin_choose_expr(1, 1, 0), "Incorrect semantics of __builtin_choose_expr"); +_Static_assert(__builtin_choose_expr(0, 0, 1), "Incorrect semantics of __builtin_choose_expr"); Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp =================================================================== --- clang/lib/ASTMatchers/Dynamic/Registry.cpp +++ clang/lib/ASTMatchers/Dynamic/Registry.cpp @@ -147,6 +147,7 @@ REGISTER_MATCHER(caseStmt); REGISTER_MATCHER(castExpr); REGISTER_MATCHER(characterLiteral); + REGISTER_MATCHER(chooseExpr); REGISTER_MATCHER(classTemplateDecl); REGISTER_MATCHER(classTemplateSpecializationDecl); REGISTER_MATCHER(complexType); Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp =================================================================== --- clang/lib/ASTMatchers/ASTMatchersInternal.cpp +++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp @@ -727,6 +727,7 @@ compoundLiteralExpr; const internal::VariadicDynCastAllOfMatcher<Stmt, CXXNullPtrLiteralExpr> cxxNullPtrLiteralExpr; +const internal::VariadicDynCastAllOfMatcher<Stmt, ChooseExpr> chooseExpr; const internal::VariadicDynCastAllOfMatcher<Stmt, GNUNullExpr> gnuNullExpr; const internal::VariadicDynCastAllOfMatcher<Stmt, AtomicExpr> atomicExpr; const internal::VariadicDynCastAllOfMatcher<Stmt, StmtExpr> stmtExpr; Index: clang/lib/AST/ASTImporter.cpp =================================================================== --- clang/lib/AST/ASTImporter.cpp +++ clang/lib/AST/ASTImporter.cpp @@ -552,6 +552,7 @@ // Importing expressions ExpectedStmt VisitExpr(Expr *E); ExpectedStmt VisitVAArgExpr(VAArgExpr *E); + ExpectedStmt VisitChooseExpr(ChooseExpr *E); ExpectedStmt VisitGNUNullExpr(GNUNullExpr *E); ExpectedStmt VisitPredefinedExpr(PredefinedExpr *E); ExpectedStmt VisitDeclRefExpr(DeclRefExpr *E); @@ -6135,6 +6136,33 @@ E->isMicrosoftABI()); } +ExpectedStmt ASTNodeImporter::VisitChooseExpr(ChooseExpr *E) { + auto Imp = importSeq(E->getCond(), E->getLHS(), E->getRHS(), + E->getBuiltinLoc(), E->getRParenLoc(), E->getType()); + if (!Imp) + return Imp.takeError(); + + Expr *ToCond; + Expr *ToLHS; + Expr *ToRHS; + SourceLocation ToBuiltinLoc, ToRParenLoc; + QualType ToType; + std::tie(ToCond, ToLHS, ToRHS, ToBuiltinLoc, ToRParenLoc, ToType) = *Imp; + + ExprValueKind VK = E->getValueKind(); + ExprObjectKind OK = E->getObjectKind(); + + bool TypeDependent = ToCond->isTypeDependent(); + bool ValueDependent = ToCond->isValueDependent(); + + // The value of CondIsTrue only matters if the value is not + // condition-dependent. + bool CondIsTrue = !E->isConditionDependent() && E->isConditionTrue(); + + return new (Importer.getToContext()) + ChooseExpr(ToBuiltinLoc, ToCond, ToLHS, ToRHS, ToType, VK, OK, + ToRParenLoc, CondIsTrue, TypeDependent, ValueDependent); +} ExpectedStmt ASTNodeImporter::VisitGNUNullExpr(GNUNullExpr *E) { ExpectedType TypeOrErr = import(E->getType()); Index: clang/include/clang/ASTMatchers/ASTMatchers.h =================================================================== --- clang/include/clang/ASTMatchers/ASTMatchers.h +++ clang/include/clang/ASTMatchers/ASTMatchers.h @@ -2158,6 +2158,10 @@ extern const internal::VariadicDynCastAllOfMatcher<Stmt, CXXNullPtrLiteralExpr> cxxNullPtrLiteralExpr; +/// Matches GNU __builtin_choose_expr. +extern const internal::VariadicDynCastAllOfMatcher<Stmt, ChooseExpr> + chooseExpr; + /// Matches GNU __null expression. extern const internal::VariadicDynCastAllOfMatcher<Stmt, GNUNullExpr> gnuNullExpr; Index: clang/docs/LibASTMatchersReference.html =================================================================== --- clang/docs/LibASTMatchersReference.html +++ clang/docs/LibASTMatchersReference.html @@ -788,6 +788,11 @@ </pre></td></tr> +<tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('chooseExpr0')"><a name="chooseExpr0Anchor">chooseExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1ChooseExpr.html">ChooseExpr</a>>...</td></tr> +<tr><td colspan="4" class="doc" id="chooseExpr0"><pre>Matches GNU __builtin_choose_expr. +</pre></td></tr> + + <tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Stmt.html">Stmt</a>></td><td class="name" onclick="toggle('compoundLiteralExpr0')"><a name="compoundLiteralExpr0Anchor">compoundLiteralExpr</a></td><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1CompoundLiteralExpr.html">CompoundLiteralExpr</a>>...</td></tr> <tr><td colspan="4" class="doc" id="compoundLiteralExpr0"><pre>Matches compound (i.e. non-scalar) literals
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits