[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL318998: [ASTImporter] Support TypeTraitExpr (authored by 
a.sidorin).

Changed prior to commit:
  https://reviews.llvm.org/D39722?vs=124286=124309#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39722

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -291,6 +291,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5890,6 +5891,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); }"
+ "};"
+ "void f() { declToImport().m(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(
+   has(
+ cxxRecordDecl(
+   has(
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(
+   hasType(booleanType())
+   )));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang


Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -291,6 +291,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5890,6 +5891,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin accepted this revision.
a.sidorin added a comment.

Thank you!


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 124286.
tk1012 added a comment.

Hello there,

I update the diff to follow your comments.


https://reviews.llvm.org/D39722

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); }"
+ "};"
+ "void f() { declToImport().m(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(
+   has(
+ cxxRecordDecl(
+   has(
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(
+   hasType(booleanType())
+   )));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); }"
+ "};"
+ "void f() { declToImport().m(); 

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39722#934783, @aaron.ballman wrote:

> In https://reviews.llvm.org/D39722#934781, @tk1012 wrote:
>
> > Hello Aaron,
> >
> > I remove the semicolon.
> >
> > > Is this type actually correct for C++?
> >
> > Yes, it is.
> >  clang generates the AST for `declToImport` struct like this.
> >
> >   |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct 
> > declToImport
> >   `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
> > `-CompoundStmt 0x8b1a1e8 
> >   `-TypeTraitExpr 0x8b1a1c8  '_Bool'
> >   
>
>
> Hmm, that looks like a bug to me; but not one that should impact this review.


Yeah, it seems as though the ASTDumper does not receive a printing policy, and 
the default behavior of `QualType::getAsString()` is to gin up a very dumb 
printing policy from default language options, hence the poor quality of the 
name here.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

In https://reviews.llvm.org/D39722#934781, @tk1012 wrote:

> Hello Aaron,
>
> I remove the semicolon.
>
> > Is this type actually correct for C++?
>
> Yes, it is.
>  clang generates the AST for `declToImport` struct like this.
>
>   |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct 
> declToImport
>   `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
> `-CompoundStmt 0x8b1a1e8 
>   `-TypeTraitExpr 0x8b1a1c8  '_Bool'
>   


Hmm, that looks like a bug to me; but not one that should impact this review.

LGTM!


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 124232.
tk1012 added a comment.

Hello Aaron,

I remove the semicolon.

> Is this type actually correct for C++?

Yes, it is.
clang generates the AST for `declToImport` struct like this.

  |-CXXRecordDecl 0x8b19fe0  col:29 implicit struct declToImport
  `-CXXMethodDecl 0x8b1a0d0  col:8 m 'void (void)'
`-CompoundStmt 0x8b1a1e8 
  `-TypeTraitExpr 0x8b1a1c8  '_Bool'




https://reviews.llvm.org/D39722

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); }"
+ "};"
+ "void f() { declToImport().m(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(
+   has(
+ cxxRecordDecl(
+   has(
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(
+   hasType(asString("_Bool"))
+   )));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+   

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: unittests/AST/ASTImporterTest.cpp:553
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); };"
+ "};"

Drop the spurious semicolon at the end of the function definition.



Comment at: unittests/AST/ASTImporterTest.cpp:566
+ typeTraitExpr(
+   hasType(asString("_Bool"))
+   )));

Is this type actually correct for C++? I would expect that for C code, but not 
for C++.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 124124.
tk1012 added a comment.

Hello there,

I update the diff and add `void f() { declToImport().m(); }` after 
`declToImport` definition.
I leave the matcher for private in the test file, so I don't update the 
documentation.


https://reviews.llvm.org/D39722

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"
+ "  void m() { __is_pod(T); };"
+ "};"
+ "void f() { declToImport().m(); }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ classTemplateDecl(
+   has(
+ cxxRecordDecl(
+   has(
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(
+   hasType(asString("_Bool"))
+   )));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,26 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgs(E->getNumArgs());
+  if (ImportContainerChecked(E->getArgs(), ToArgs))
+return nullptr;
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = false;
+  if (!E->isValueDependent())
+ToValue = E->getValue();
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgs, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,47 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
+
+TEST(ImportExpr, ImportTypeTraitExprValDep) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("template struct declToImport {"

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39722#933838, @xazax.hun wrote:

> In https://reviews.llvm.org/D39722#933828, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D39722#933699, @a.sidorin wrote:
> >
> > > Hello Takafumi,
> > >
> > > This is almost OK to me but there is an inline comment we need to resolve 
> > > in order to avoid Windows buildbot failures.
> > >  In addition, as Gabor pointed, when we add a new matcher, we need to 
> > > update matcher documentation as well. To update the docs, you should 
> > > perform just three steps.
> > >
> > > 1. Rebase your patch onto latest master
> > > 2. Launch script docs/tools/dump_ast_matchers.py
> > > 3. Add changes made by this script to your commit.
> >
> >
> > I'm a bit confused -- I don't see any ASTMatcher changes as part of this 
> > patch aside from what's in the test file. Are you suggesting the matcher 
> > there should be hoisted as a separate patch?
>
>
> In a previous version the matcher was public. I think it either should be 
> public with documentation or it can remain in the test file and no doc update 
> required. I am fine with both solution.


I'm fine with either exposing it as a public matcher or leaving it private for 
the test file, but if it gets exposed, I think it should be in a separate patch.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Oh, sorry! I was thinking that a matcher is still in ASTMatchers.h (in previous 
revisions it was there). If matcher is internal (current revision), there is no 
need to update docs.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In https://reviews.llvm.org/D39722#933828, @aaron.ballman wrote:

> In https://reviews.llvm.org/D39722#933699, @a.sidorin wrote:
>
> > Hello Takafumi,
> >
> > This is almost OK to me but there is an inline comment we need to resolve 
> > in order to avoid Windows buildbot failures.
> >  In addition, as Gabor pointed, when we add a new matcher, we need to 
> > update matcher documentation as well. To update the docs, you should 
> > perform just three steps.
> >
> > 1. Rebase your patch onto latest master
> > 2. Launch script docs/tools/dump_ast_matchers.py
> > 3. Add changes made by this script to your commit.
>
>
> I'm a bit confused -- I don't see any ASTMatcher changes as part of this 
> patch aside from what's in the test file. Are you suggesting the matcher 
> there should be hoisted as a separate patch?


In a previous version the matcher was public. I think it either should be 
public with documentation or it can remain in the test file and no doc update 
required. I am fine with both solution.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D39722#933699, @a.sidorin wrote:

> Hello Takafumi,
>
> This is almost OK to me but there is an inline comment we need to resolve in 
> order to avoid Windows buildbot failures.
>  In addition, as Gabor pointed, when we add a new matcher, we need to update 
> matcher documentation as well. To update the docs, you should perform just 
> three steps.
>
> 1. Rebase your patch onto latest master
> 2. Launch script docs/tools/dump_ast_matchers.py
> 3. Add changes made by this script to your commit.


I'm a bit confused -- I don't see any ASTMatcher changes as part of this patch 
aside from what's in the test file. Are you suggesting the matcher there should 
be hoisted as a separate patch?


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Takafumi,

This is almost OK to me but there is an inline comment we need to resolve in 
order to avoid Windows buildbot failures.
In addition, as Gabor pointed, when we add a new matcher, we need to update 
matcher documentation as well. To update the docs, you should perform just 
three steps.

1. Rebase your patch onto latest master
2. Launch script docs/tools/dump_ast_matchers.py
3. Add changes made by this script to your commit.

After these changes are made, I will approve the patch. Thank you!




Comment at: unittests/AST/ASTImporterTest.cpp:554
+ "  void m() { __is_pod(T); };"
+ "};",
+ Lang_CXX11, "", Lang_CXX11, Verifier,

Please add `void f() { declToImport().m(); } after `declToImport` 
definition. The reason is that in MSVC mode, uninstantiated templates are 
ignored so the test will fail. We need this to avoid this.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5622
+  SmallVector ToArgVec;
+  for (auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);

aaron.ballman wrote:
> `const auto *`?
By the way, you can replace the loop with `ImportContainerChecked()`.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5622
+  SmallVector ToArgVec;
+  for (auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);

`const auto *`?



Comment at: lib/AST/ASTImporter.cpp:5631
+  // Value is always false.
+  bool ToValue = (!E->isValueDependent()) ? E->getValue() : false;
+

Remove spurious parens.



Comment at: unittests/AST/ASTImporterTest.cpp:548
+ typeTraitExpr(hasType(asString("int");
+}
 

Please add a value-dependent test as well.


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 123792.
tk1012 added a comment.

Hello Gábor,

Thank you for responding.

I move the definition of the mather `typeTraitExpr` into 
unittests/AST/ASTImporterTest.cpp.
I also slightly modify the test code.


https://reviews.llvm.org/D39722

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,27 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,28 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgVec;
+  for (auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);
+if (!ToTI)
+  return nullptr;
+ToArgVec.push_back(ToTI);
+  }
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = (!E->isValueDependent()) ? E->getValue() : false;
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgVec, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,27 @@
  declRefExpr()));
 }
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { "
+ "  __builtin_types_compatible_p(int, int);"
+ "}",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr(hasType(asString("int");
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,28 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgVec;
+  for (auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);
+if (!ToTI)
+  return nullptr;
+ToArgVec.push_back(ToTI);
+  }
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value 

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.

The checker documentation should be updated as well.




Comment at: include/clang/ASTMatchers/ASTMatchers.h:2251
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+

Note that the matchers are no longer defined in the header. They are just 
declared and the definition is moved to a cpp file. 


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Aaron,

This patch is OK for me but could you please take a look at ASTMatchers changes?


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin accepted this revision.
a.sidorin added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 added a comment.

Description of the Sema::BuildTypeTrait()




Comment at: lib/AST/ASTImporter.cpp:5631
+  // Value is always false.
+  bool ToValue = (!E->isValueDependent()) ? E->getValue() : false;
+

According to Sema::BuildTypeTrait() in lib/Sema/SemaExprCXX.cpp, 
if an argument of TypeTraitExpr has a dependent type,
TypeTraitExpr's value (`Result`) is always false.

```
ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,

 bool Dependent = false;
 for (unsigned I = 0, N = Args.size(); I != N; ++I) {
   if (Args[I]->getType()->isDependentType()) {
 Dependent = true;
 break;
   }
 }

 bool Result = false;
 if (!Dependent)
   Result = evaluateTypeTrait(*this, Kind, KWLoc, Args, RParenLoc);

 return TypeTraitExpr::Create(Context, ResultType, KWLoc, Kind, Args,
  RParenLoc, Result);
...

```


https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 marked 3 inline comments as done.
tk1012 added a comment.




https://reviews.llvm.org/D39722



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

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Takafumi Kubota via Phabricator via cfe-commits]

tk1012 updated this revision to Diff 123539.
tk1012 added a comment.
Herald added a subscriber: rnkovacs.

Hello, Aleksei.

I'm sorry for the long response time.
I update the diff to follow your comments.

Updates

1. I apply clang-format -style=llvm to ASTMatchers.h and ASTImporter.cpp. I 
don't apply it to ASTImporterTest.cpp for readability of the matching types.
2. I add `return nullptr;` if `ToTI` is `nullptr`.
3. I directly pass `ToArgVec` to `TypeTraitExpr::Create()`
4. I add a check for the value-dependent of `E'.




https://reviews.llvm.org/D39722

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,16 @@
  declRefExpr()));
 }
 
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { 
__builtin_types_compatible_p(int, int); }",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr()));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,28 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgVec;
+  for (auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);
+if (!ToTI)
+  return nullptr;
+ToArgVec.push_back(ToTI);
+  }
+
+  // According to Sema::BuildTypeTrait(), if E is value-dependent,
+  // Value is always false.
+  bool ToValue = (!E->isValueDependent()) ? E->getValue() : false;
+
+  return TypeTraitExpr::Create(
+  Importer.getToContext(), ToType, Importer.Import(E->getLocStart()),
+  E->getTrait(), ToArgVec, Importer.Import(E->getLocEnd()), ToValue);
+}
+
 void ASTNodeImporter::ImportOverrides(CXXMethodDecl *ToMethod,
   CXXMethodDecl *FromMethod) {
   for (auto *FromOverriddenMethod : FromMethod->overridden_methods())
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2241,6 +2241,15 @@
 2, std::numeric_limits::max()>
 allOf;
 
+/// \brief Matches __builtin_types_compatible_p:
+/// GNU extension to check equivalent types
+/// Given
+/// \code
+///   __builtin_types_compatible_p(int, int)
+/// \endcode
+//  will generate TypeTraitExpr <...> 'int'
+const internal::VariadicDynCastAllOfMatcher typeTraitExpr;
+
 /// \brief Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
 ///
 /// Given


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -525,6 +525,16 @@
  declRefExpr()));
 }
 
+TEST(ImportExpr, ImportTypeTraitExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { __builtin_types_compatible_p(int, int); }",
+ Lang_C, "", Lang_C, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ typeTraitExpr()));
+}
 
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -283,6 +283,7 @@
 Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E);
 Expr *VisitCXXNamedCastExpr(CXXNamedCastExpr *E);
 Expr *VisitSubstNonTypeTemplateParmExpr(SubstNonTypeTemplateParmExpr *E);
+Expr *VisitTypeTraitExpr(TypeTraitExpr *E);
 
 
 template
@@ -5612,6 +5613,28 @@
 Replacement);
 }
 
+Expr *ASTNodeImporter::VisitTypeTraitExpr(TypeTraitExpr *E) {
+  QualType ToType = Importer.Import(E->getType());
+  if (ToType.isNull())
+return nullptr;
+
+  SmallVector ToArgVec;
+  for (auto FromArg : 

[PATCH] D39722: [ASTImporter] Support TypeTraitExpr Importing

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hello Takafumi,

Thank you for this patch! I feel positive about it. You can find my comments 
inline.




Comment at: lib/AST/ASTImporter.cpp:5540
+  for(auto FromArg : E->getArgs()) {
+TypeSourceInfo *ToTI = Importer.Import(FromArg);
+ToArgVec.push_back(ToTI);

We should fail (`return nullptr`) if import fails and `ToTI` is `nullptr`.



Comment at: lib/AST/ASTImporter.cpp:5543
+  }
+  ArrayRef ToArgRef(ToArgVec);
+

No need to create an ArrayRef - SmallVector can be implicitly converted to it 
so you can just pass SmallVector to the function.



Comment at: lib/AST/ASTImporter.cpp:5545
+
+  return TypeTraitExpr::Create( Importer.getToContext(),
+ ToType,

The style looks a bit broken. Could you clang-format?



Comment at: lib/AST/ASTImporter.cpp:5551
+ Importer.Import(E->getLocEnd()),
+ E->getValue());
+}

This will assert if E is value-dependent. You need to check it explicitly.


https://reviews.llvm.org/D39722



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