[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-26 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

Sounds good, thanks for debugging this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-26 Thread Tom Roeder via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL354916: [ASTImporter] Add support for importing ChooseExpr 
AST nodes. (authored by tmroeder, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58663?vs=188397=188425#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58663

Files:
  cfe/trunk/docs/LibASTMatchersReference.html
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
  cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
  cfe/trunk/test/ASTMerge/choose-expr/Inputs/choose.c
  cfe/trunk/test/ASTMerge/choose-expr/test.c
  cfe/trunk/unittests/AST/ASTImporterTest.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/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: cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -727,6 +727,7 @@
 compoundLiteralExpr;
 const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
+++ cfe/trunk/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: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -563,6 +563,17 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier 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 Verifier;
   testImport(
@@ -1312,6 +1323,29 @@
   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 

[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-26 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder added a comment.

I'm going to submit this patch again, since that I believe I understand the 
problem, and I have tested this version on Win10 with the latest MSVC (other 
than the expectation that I just added, but that test wasn't a problem on the 
Windows builders, and the new expectation passes on my Linux dev box). I'll 
watch the Windows build at 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc and revert if there 
are problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-26 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder updated this revision to Diff 188397.
tmroeder added a comment.

Added the other expectation, as suggested.


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 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 Verifier;
   testImport(
@@ -1312,6 +1323,29 @@
   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("choose", FromResults);
+  ASSERT_TRUE(FromChooseExpr);
+
+  const ChooseExpr *ToChooseExpr = selectFirst("choose", ToResults);
+  ASSERT_TRUE(ToChooseExpr);
+
+  EXPECT_EQ(FromChooseExpr->isConditionTrue(), ToChooseExpr->isConditionTrue());
+  EXPECT_EQ(FromChooseExpr->isConditionDependent(),
+ToChooseExpr->isConditionDependent());
+}
+
 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
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 const internal::VariadicDynCastAllOfMatcher atomicExpr;
 const internal::VariadicDynCastAllOfMatcher stmtExpr;
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp

[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-26 Thread Gabor Marton via Phabricator via cfe-commits
martong added inline comments.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:1344
+
+  EXPECT_EQ(FromChooseExpr->isConditionTrue(), 
ToChooseExpr->isConditionTrue());
+}

To  compensate the  skipping of the template test, perhaps we should have 
another expectation for the condition dependency of the expression:
```
EXPECT_EQ(FromChooseExpr->isConditionDependent(), 
ToChooseExpr->isConditionDependent());
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58663



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-25 Thread Tom Roeder via Phabricator via cfe-commits
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 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 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("choose", FromResults);
+  ASSERT_TRUE(FromChooseExpr);
+
+  const ChooseExpr *ToChooseExpr = selectFirst("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
 cxxNullPtrLiteralExpr;
+const internal::VariadicDynCastAllOfMatcher chooseExpr;
 const internal::VariadicDynCastAllOfMatcher gnuNullExpr;
 

[PATCH] D58663: [ASTImporter] Add support for importing ChooseExpr AST nodes.

2019-02-25 Thread Tom Roeder via Phabricator via cfe-commits
tmroeder created this revision.
tmroeder added reviewers: shafik, a_sidorin, martong, aaron.ballman, rnk.
tmroeder added a project: clang.
Herald added a reviewer: a.sidorin.

This allows ASTs to be merged when they contain ChooseExpr (the GNU
__builtin_choose_expr construction). This is needed, for example, for
cross-CTU analysis of C code that makes use of __builtin_choose_expr.

The node is already supported in the AST, but it didn't have a matcher
in ASTMatchers. So, this change adds the matcher and adds support to
ASTImporter.

This was originally reviewed and approved in
https://reviews.llvm.org/D58292 and submitted as r354832. It was
reverted in r354839 due to failures on the Windows CI builds.

This version fixes the test failures on Windows, caused by differences
in the behavior of -fms-compatibility between Linux and MSVC builds. In
MSVC builds, -fms-compatibility delays template evaluation for the C++
test case in clang/unittests/AST/ASTImporter.cpp, and that causes the
matcher to fail to match chooseExpr().

The only difference between this patch and r354839 is that the unit test
now checks for "-fms-compatibility" and treats it like
"-fdelayed-template-parsing": it doesn't try to match chooseExpr().


Repository:
  rG LLVM Github Monorepo

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,36 @@
   stringLiteral(hasType(asString("const char [7]"));
 }
 
+TEST_P(ImportExpr, ImportChooseExpr) {
+  MatchVerifier 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(;
+
+  ArgVector Args = getExtraArgs();
+  BindableMatcher Matcher =
+  functionTemplateDecl(hasDescendant(chooseExpr()));
+
+  // Don't try to match the template contents if template parsing. The
+  // -fms-compatibility flag delays template parsing on Windows.
+  if (llvm::find(Args, "-fdelayed-template-parsing") != Args.end() ||
+  llvm::find(Args, "-fms-compatibility") != Args.end()) {
+Matcher = functionTemplateDecl();
+  }
+
+  // Make sure that uses of (void)__builtin_choose_expr with dependent types in
+  // the condition are handled properly. This test triggers an assertion if the
+  // ASTImporter incorrectly tries to access isConditionTrue() when
+  // isConditionDependent() is true.
+  testImport("template void declToImport() { "
+ "(void)__builtin_choose_expr(N, 1, 0); }",
+ Lang_CXX, "", Lang_CXX, Verifier, Matcher);
+}
+
 TEST_P(ImportExpr, ImportGNUNullExpr) {
   MatchVerifier Verifier;
   testImport(
@@ -1312,6 +1342,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("choose", FromResults);
+  ASSERT_TRUE(FromChooseExpr);
+
+  const ChooseExpr *ToChooseExpr = selectFirst("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
+++