r370461 - [ASTImporter] Do not look up lambda classes

Gabor Marton via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 30 03:55:41 PDT 2019


Author: martong
Date: Fri Aug 30 03:55:41 2019
New Revision: 370461

URL: http://llvm.org/viewvc/llvm-project?rev=370461&view=rev
Log:
[ASTImporter] Do not look up lambda classes

Summary:
Consider this code:
```
      void f() {
        auto L0 = [](){};
        auto L1 = [](){};
      }

```
First we import `L0` then `L1`. Currently we end up having only one
CXXRecordDecl for the two different lambdas. And that is a problem if
the body of their op() is different. This happens because when we import
`L1` then lookup finds the existing `L0` and since they are structurally
equivalent we just map the imported L0 to be the counterpart of L1.

We have the same problem in this case:
```
      template <typename F0, typename F1>
      void f(F0 L0 = [](){}, F1 L1 = [](){}) {}

```

In StructuralEquivalenceContext we could distinquish lambdas only by
their source location in these cases. But we the lambdas are actually
structrually equivalent they differn only by the source location.

Thus, the  solution is to disable lookup completely if the decl in
the "from" context is a lambda.
However, that could have other problems: what if the lambda is defined
in a header file and included in several TUs? I think we'd have as many
duplicates as many includes we have. I think we could live with that,
because the lambda classes are TU local anyway, we cannot just access
them from another TU.

Reviewers: a_sidorin, a.sidorin, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D66348

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

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=370461&r1=370460&r2=370461&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Aug 30 03:55:41 2019
@@ -2633,7 +2633,7 @@ ExpectedDecl ASTNodeImporter::VisitRecor
 
   // We may already have a record of the same name; try to find and match it.
   RecordDecl *PrevDecl = nullptr;
-  if (!DC->isFunctionOrMethod()) {
+  if (!DC->isFunctionOrMethod() && !D->isLambda()) {
     SmallVector<NamedDecl *, 4> ConflictingDecls;
     auto FoundDecls =
         Importer.findDeclsInToCtx(DC, SearchName);

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=370461&r1=370460&r2=370461&view=diff
==============================================================================
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Aug 30 03:55:41 2019
@@ -5644,6 +5644,117 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTes
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, CanonicalRedeclChain,
                         ::testing::Values(ArgVector()), );
 
+TEST_P(ASTImporterOptionSpecificTestBase, LambdasAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      void f() {
+        auto L0 = [](){};
+        auto L1 = [](){};
+      }
+      )",
+      Lang_CXX11, "input0.cc");
+  auto Pattern = lambdaExpr();
+  CXXRecordDecl *FromL0 =
+      FirstDeclMatcher<LambdaExpr>().match(FromTU, Pattern)->getLambdaClass();
+  CXXRecordDecl *FromL1 =
+      LastDeclMatcher<LambdaExpr>().match(FromTU, Pattern)->getLambdaClass();
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  EXPECT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       LambdasInFunctionParamsAreDifferentiated) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <typename F0, typename F1>
+      void f(F0 L0 = [](){}, F1 L1 = [](){}) {}
+      )",
+      Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+      FirstDeclMatcher<CXXRecordDecl>().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+      LastDeclMatcher<CXXRecordDecl>().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+       LambdasInFunctionParamsAreDifferentiatedWhenMacroIsUsed) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      #define LAMBDA [](){}
+      template <typename F0, typename F1>
+      void f(F0 L0 = LAMBDA, F1 L1 = LAMBDA) {}
+      )",
+      Lang_CXX11, "input0.cc");
+  auto Pattern = cxxRecordDecl(isLambda());
+  CXXRecordDecl *FromL0 =
+      FirstDeclMatcher<CXXRecordDecl>().match(FromTU, Pattern);
+  CXXRecordDecl *FromL1 =
+      LastDeclMatcher<CXXRecordDecl>().match(FromTU, Pattern);
+  ASSERT_NE(FromL0, FromL1);
+
+  Import(FromL0, Lang_CXX11);
+  Import(FromL1, Lang_CXX11);
+  CXXRecordDecl *ToL0 = Import(FromL0, Lang_CXX11);
+  CXXRecordDecl *ToL1 = Import(FromL1, Lang_CXX11);
+  ASSERT_NE(ToL0, ToL1);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportAssignedLambda) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      void f() {
+        auto x = []{} = {}; auto x2 = x;
+      }
+      )",
+      Lang_CXX2a, "input0.cc");
+  auto FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f")));
+  // We have only one lambda class.
+  ASSERT_EQ(
+      DeclCounter<CXXRecordDecl>().match(FromTU, cxxRecordDecl(isLambda())),
+      1u);
+
+  FunctionDecl *ToF = Import(FromF, Lang_CXX2a);
+  EXPECT_TRUE(ToF);
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We have only one lambda class after the import.
+  EXPECT_EQ(DeclCounter<CXXRecordDecl>().match(ToTU, cxxRecordDecl(isLambda())),
+            1u);
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportDefaultConstructibleLambdas) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      void f() {
+        auto x = []{} = {};
+        auto xb = []{} = {};
+      }
+      )",
+      Lang_CXX2a, "input0.cc");
+  auto FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f")));
+  // We have two lambda classes.
+  ASSERT_EQ(
+      DeclCounter<CXXRecordDecl>().match(FromTU, cxxRecordDecl(isLambda())),
+      2u);
+
+  FunctionDecl *ToF = Import(FromF, Lang_CXX2a);
+  EXPECT_TRUE(ToF);
+  TranslationUnitDecl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We have two lambda classes after the import.
+  EXPECT_EQ(DeclCounter<CXXRecordDecl>().match(ToTU, cxxRecordDecl(isLambda())),
+            2u);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
                         DefaultTestValuesForRunOptions, );
 




More information about the cfe-commits mailing list