From 940667de80ecc16952d0add91261e565b9466f37 Mon Sep 17 00:00:00 2001 From: Gabor Marton Date: Fri, 30 Aug 2019 10:55:41 +0000 Subject: [PATCH] [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 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@370461 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/ASTImporter.cpp | 2 +- unittests/AST/ASTImporterTest.cpp | 111 ++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index 30ec1374ae..5ee9f760b6 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -2633,7 +2633,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { // 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 ConflictingDecls; auto FoundDecls = Importer.findDeclsInToCtx(DC, SearchName); diff --git a/unittests/AST/ASTImporterTest.cpp b/unittests/AST/ASTImporterTest.cpp index 2c27a4417f..cfee284e68 100644 --- a/unittests/AST/ASTImporterTest.cpp +++ b/unittests/AST/ASTImporterTest.cpp @@ -5644,6 +5644,117 @@ INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest, 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().match(FromTU, Pattern)->getLambdaClass(); + CXXRecordDecl *FromL1 = + LastDeclMatcher().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 + void f(F0 L0 = [](){}, F1 L1 = [](){}) {} + )", + Lang_CXX11, "input0.cc"); + auto Pattern = cxxRecordDecl(isLambda()); + CXXRecordDecl *FromL0 = + FirstDeclMatcher().match(FromTU, Pattern); + CXXRecordDecl *FromL1 = + LastDeclMatcher().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 + void f(F0 L0 = LAMBDA, F1 L1 = LAMBDA) {} + )", + Lang_CXX11, "input0.cc"); + auto Pattern = cxxRecordDecl(isLambda()); + CXXRecordDecl *FromL0 = + FirstDeclMatcher().match(FromTU, Pattern); + CXXRecordDecl *FromL1 = + LastDeclMatcher().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().match( + FromTU, functionDecl(hasName("f"))); + // We have only one lambda class. + ASSERT_EQ( + DeclCounter().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().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().match( + FromTU, functionDecl(hasName("f"))); + // We have two lambda classes. + ASSERT_EQ( + DeclCounter().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().match(ToTU, cxxRecordDecl(isLambda())), + 2u); +} + INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest, DefaultTestValuesForRunOptions, ); -- 2.40.0