maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[ASTImporter] Check visibility/linkage of functions and variables 

Summary:
During import of a global variable with external visibility the lookup
will find variables (with the same name) but with static visibility.
Clearly, we cannot put them into the same redecl chain.  The same is
true in case of functions.  In this fix we filter the lookup results and
consider only those which have the same visibility as the decl we
currently import.

We consider two decls in two anonymous namsepaces to have the same
visibility only if they are imported from the very same translation
unit.

Reviewers: a_sidorin, shafik, a.sidorin

Reviewed By: shafik

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

Tags: #clang

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

llvm-svn: 354027
This commit is contained in:
Gabor Marton 2019-02-14 13:07:03 +00:00
parent 15e475e222
commit 458d1457fb
3 changed files with 370 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
clang/include/clang/AST/ASTImporter.h
View File

@ -32,6 +32,7 @@
namespace clang {
class ASTContext;
class Attr;
class ASTImporterLookupTable;
class CXXBaseSpecifier;
class CXXCtorInitializer;
@ -42,8 +43,8 @@ class FileManager;
class NamedDecl;
class Stmt;
class TagDecl;
class TranslationUnitDecl;
class TypeSourceInfo;
class Attr;
class ImportError : public llvm::ErrorInfo<ImportError> {
public:
@ -115,6 +116,10 @@ class Attr;
/// context to the corresponding declarations in the "to" context.
llvm::DenseMap<Decl *, Decl *> ImportedDecls;
/// Mapping from the already-imported declarations in the "to"
/// context to the corresponding declarations in the "from" context.
llvm::DenseMap<Decl *, Decl *> ImportedFromDecls;
/// Mapping from the already-imported statements in the "from"
/// context to the corresponding statements in the "to" context.
llvm::DenseMap<Stmt *, Stmt *> ImportedStmts;
@ -226,9 +231,13 @@ class Attr;
/// Return the copy of the given declaration in the "to" context if
/// it has already been imported from the "from" context. Otherwise return
/// NULL.
/// nullptr.
Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
/// Return the translation unit from where the declaration was
/// imported. If it does not exist nullptr is returned.
TranslationUnitDecl *GetFromTU(Decl *ToD);
/// Import the given declaration context from the "from"
/// AST context into the "to" AST context.
///

161
clang/lib/AST/ASTImporter.cpp
View File

@ -439,6 +439,9 @@ namespace clang {
Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
template <typename T>
bool hasSameVisibilityContext(T *Found, T *From);
bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
@ -2957,6 +2960,19 @@ Error ASTNodeImporter::ImportFunctionDeclBody(FunctionDecl *FromFD,
return Error::success();
}
template <typename T>
bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
if (From->hasExternalFormalLinkage())
return Found->hasExternalFormalLinkage();
if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
return false;
if (From->isInAnonymousNamespace())
return Found->isInAnonymousNamespace();
else
return !Found->isInAnonymousNamespace() &&
!Found->hasExternalFormalLinkage();
}
ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
SmallVector<Decl *, 2> Redecls = getCanonicalForwardRedeclChain(D);
@ -3010,33 +3026,30 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
continue;
if (auto *FoundFunction = dyn_cast<FunctionDecl>(FoundDecl)) {
if (FoundFunction->hasExternalFormalLinkage() &&
D->hasExternalFormalLinkage()) {
if (IsStructuralMatch(D, FoundFunction)) {
const FunctionDecl *Definition = nullptr;
if (D->doesThisDeclarationHaveABody() &&
FoundFunction->hasBody(Definition)) {
return Importer.MapImported(
D, const_cast<FunctionDecl *>(Definition));
}
FoundByLookup = FoundFunction;
break;
}
if (!hasSameVisibilityContext(FoundFunction, D))
continue;
// FIXME: Check for overloading more carefully, e.g., by boosting
// Sema::IsOverload out to the AST library.
// Function overloading is okay in C++.
if (Importer.getToContext().getLangOpts().CPlusPlus)
continue;
// Complain about inconsistent function types.
Importer.ToDiag(Loc, diag::err_odr_function_type_inconsistent)
<< Name << D->getType() << FoundFunction->getType();
Importer.ToDiag(FoundFunction->getLocation(),
diag::note_odr_value_here)
<< FoundFunction->getType();
if (IsStructuralMatch(D, FoundFunction)) {
const FunctionDecl *Definition = nullptr;
if (D->doesThisDeclarationHaveABody() &&
FoundFunction->hasBody(Definition))
return Importer.MapImported(D,
const_cast<FunctionDecl *>(Definition));
FoundByLookup = FoundFunction;
break;
}
// FIXME: Check for overloading more carefully, e.g., by boosting
// Sema::IsOverload out to the AST library.
// Function overloading is okay in C++.
if (Importer.getToContext().getLangOpts().CPlusPlus)
continue;
// Complain about inconsistent function types.
Importer.ToDiag(Loc, diag::err_odr_function_type_inconsistent)
<< Name << D->getType() << FoundFunction->getType();
Importer.ToDiag(FoundFunction->getLocation(), diag::note_odr_value_here)
<< FoundFunction->getType();
}
ConflictingDecls.push_back(FoundDecl);
@ -3609,58 +3622,56 @@ ExpectedDecl ASTNodeImporter::VisitVarDecl(VarDecl *D) {
continue;
if (auto *FoundVar = dyn_cast<VarDecl>(FoundDecl)) {
// We have found a variable that we may need to merge with. Check it.
if (FoundVar->hasExternalFormalLinkage() &&
D->hasExternalFormalLinkage()) {
if (Importer.IsStructurallyEquivalent(D->getType(),
FoundVar->getType())) {
if (!hasSameVisibilityContext(FoundVar, D))
continue;
if (Importer.IsStructurallyEquivalent(D->getType(),
FoundVar->getType())) {
// The VarDecl in the "From" context has a definition, but in the
// "To" context we already have a definition.
VarDecl *FoundDef = FoundVar->getDefinition();
if (D->isThisDeclarationADefinition() && FoundDef)
// FIXME Check for ODR error if the two definitions have
// different initializers?
return Importer.MapImported(D, FoundDef);
// The VarDecl in the "From" context has a definition, but in the
// "To" context we already have a definition.
VarDecl *FoundDef = FoundVar->getDefinition();
if (D->isThisDeclarationADefinition() && FoundDef)
// FIXME Check for ODR error if the two definitions have
// different initializers?
return Importer.MapImported(D, FoundDef);
// The VarDecl in the "From" context has an initializer, but in the
// "To" context we already have an initializer.
const VarDecl *FoundDInit = nullptr;
if (D->getInit() && FoundVar->getAnyInitializer(FoundDInit))
// FIXME Diagnose ODR error if the two initializers are different?
return Importer.MapImported(D, const_cast<VarDecl*>(FoundDInit));
// The VarDecl in the "From" context has an initializer, but in the
// "To" context we already have an initializer.
const VarDecl *FoundDInit = nullptr;
if (D->getInit() && FoundVar->getAnyInitializer(FoundDInit))
// FIXME Diagnose ODR error if the two initializers are different?
return Importer.MapImported(D, const_cast<VarDecl*>(FoundDInit));
FoundByLookup = FoundVar;
break;
}
const ArrayType *FoundArray
= Importer.getToContext().getAsArrayType(FoundVar->getType());
const ArrayType *TArray
= Importer.getToContext().getAsArrayType(D->getType());
if (FoundArray && TArray) {
if (isa<IncompleteArrayType>(FoundArray) &&
isa<ConstantArrayType>(TArray)) {
// Import the type.
if (auto TyOrErr = import(D->getType()))
FoundVar->setType(*TyOrErr);
else
return TyOrErr.takeError();
FoundByLookup = FoundVar;
break;
} else if (isa<IncompleteArrayType>(TArray) &&
isa<ConstantArrayType>(FoundArray)) {
FoundByLookup = FoundVar;
break;
}
const ArrayType *FoundArray
= Importer.getToContext().getAsArrayType(FoundVar->getType());
const ArrayType *TArray
= Importer.getToContext().getAsArrayType(D->getType());
if (FoundArray && TArray) {
if (isa<IncompleteArrayType>(FoundArray) &&
isa<ConstantArrayType>(TArray)) {
// Import the type.
if (auto TyOrErr = import(D->getType()))
FoundVar->setType(*TyOrErr);
else
return TyOrErr.takeError();
FoundByLookup = FoundVar;
break;
} else if (isa<IncompleteArrayType>(TArray) &&
isa<ConstantArrayType>(FoundArray)) {
FoundByLookup = FoundVar;
break;
}
}
Importer.ToDiag(Loc, diag::err_odr_variable_type_inconsistent)
<< Name << D->getType() << FoundVar->getType();
Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here)
<< FoundVar->getType();
}
Importer.ToDiag(Loc, diag::err_odr_variable_type_inconsistent)
<< Name << D->getType() << FoundVar->getType();
Importer.ToDiag(FoundVar->getLocation(), diag::note_odr_value_here)
<< FoundVar->getType();
}
ConflictingDecls.push_back(FoundDecl);
@ -7791,6 +7802,13 @@ Decl *ASTImporter::GetAlreadyImportedOrNull(const Decl *FromD) const {
return nullptr;
}
TranslationUnitDecl *ASTImporter::GetFromTU(Decl *ToD) {
auto FromDPos = ImportedFromDecls.find(ToD);
if (FromDPos == ImportedFromDecls.end())
return nullptr;
return FromDPos->second->getTranslationUnitDecl();
}
Expected<Decl *> ASTImporter::Import_New(Decl *FromD) {
Decl *ToD = Import(FromD);
if (!ToD && FromD)
@ -8541,6 +8559,9 @@ Decl *ASTImporter::MapImported(Decl *From, Decl *To) {
if (Pos != ImportedDecls.end())
return Pos->second;
ImportedDecls[From] = To;
// This mapping should be maintained only in this function. Therefore do not
// check for additional consistency.
ImportedFromDecls[To] = From;
return To;
}

268
clang/unittests/AST/ASTImporterTest.cpp
View File

@ -10,6 +10,10 @@
//
//===----------------------------------------------------------------------===//
// Define this to have ::testing::Combine available.
// FIXME: Better solution for this?
#define GTEST_HAS_COMBINE 1
#include "clang/AST/ASTImporter.h"
#include "MatchVerifier.h"
#include "clang/AST/ASTContext.h"
@ -461,6 +465,10 @@ public:
return FromTU->import(*LookupTablePtr, ToAST.get(), From);
}
template <class DeclT> DeclT *Import(DeclT *From, Language Lang) {
return cast_or_null<DeclT>(Import(cast<Decl>(From), Lang));
}
QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) {
lazyInitToAST(ToLang, "", OutputFileName);
TU *FromTU = findFromTU(TUDecl);
@ -2441,6 +2449,266 @@ TEST_P(ImportFunctions,
EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
}
//FIXME Move these tests to a separate test file.
namespace TypeAndValueParameterizedTests {
// Type parameters for type-parameterized test fixtures.
struct GetFunPattern {
using DeclTy = FunctionDecl;
BindableMatcher<Decl> operator()() { return functionDecl(hasName("f")); }
};
struct GetVarPattern {
using DeclTy = VarDecl;
BindableMatcher<Decl> operator()() { return varDecl(hasName("v")); }
};
// Values for the value-parameterized test fixtures.
// FunctionDecl:
auto *ExternF = "void f();";
auto *StaticF = "static void f();";
auto *AnonF = "namespace { void f(); }";
// VarDecl:
auto *ExternV = "extern int v;";
auto *StaticV = "static int v;";
auto *AnonV = "namespace { extern int v; }";
// First value in tuple: Compile options.
// Second value in tuple: Source code to be used in the test.
using ImportVisibilityChainParams =
::testing::WithParamInterface<std::tuple<ArgVector, const char *>>;
// Fixture to test the redecl chain of Decls with the same visibility. Gtest
// makes it possible to have either value-parameterized or type-parameterized
// fixtures. However, we cannot have both value- and type-parameterized test
// fixtures. This is a value-parameterized test fixture in the gtest sense. We
// intend to mimic gtest's type-parameters via the PatternFactory template
// parameter. We manually instantiate the different tests with the each types.
template <typename PatternFactory>
class ImportVisibilityChain
: public ASTImporterTestBase, public ImportVisibilityChainParams {
protected:
using DeclTy = typename PatternFactory::DeclTy;
ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
std::string getCode() const { return std::get<1>(GetParam()); }
BindableMatcher<Decl> getPattern() const { return PatternFactory()(); }
// Type-parameterized test.
void TypedTest_ImportChain() {
std::string Code = getCode() + getCode();
auto Pattern = getPattern();
TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_CXX, "input0.cc");
auto *FromF0 = FirstDeclMatcher<DeclTy>().match(FromTu, Pattern);
auto *FromF1 = LastDeclMatcher<DeclTy>().match(FromTu, Pattern);
auto *ToF0 = Import(FromF0, Lang_CXX);
auto *ToF1 = Import(FromF1, Lang_CXX);
EXPECT_TRUE(ToF0);
ASSERT_TRUE(ToF1);
EXPECT_NE(ToF0, ToF1);
EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
}
};
// Manual instantiation of the fixture with each type.
using ImportFunctionsVisibilityChain = ImportVisibilityChain<GetFunPattern>;
using ImportVariablesVisibilityChain = ImportVisibilityChain<GetVarPattern>;
// Value-parameterized test for the first type.
TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
TypedTest_ImportChain();
}
// Value-parameterized test for the second type.
TEST_P(ImportVariablesVisibilityChain, ImportChain) {
TypedTest_ImportChain();
}
// Automatic instantiation of the value-parameterized tests.
INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
::testing::Combine(
DefaultTestValuesForRunOptions,
::testing::Values(ExternF, StaticF, AnonF)), );
INSTANTIATE_TEST_CASE_P(
ParameterizedTests, ImportVariablesVisibilityChain,
::testing::Combine(
DefaultTestValuesForRunOptions,
// There is no point to instantiate with StaticV, because in C++ we can
// forward declare a variable only with the 'extern' keyword.
// Consequently, each fwd declared variable has external linkage. This
// is different in the C language where any declaration without an
// initializer is a tentative definition, subsequent definitions may be
// provided but they must have the same linkage. See also the test
// ImportVariableChainInC which test for this special C Lang case.
::testing::Values(ExternV, AnonV)), );
// First value in tuple: Compile options.
// Second value in tuple: Tuple with informations for the test.
// Code for first import (or initial code), code to import, whether the `f`
// functions are expected to be linked in a declaration chain.
// One value of this tuple is combined with every value of compile options.
// The test can have a single tuple as parameter only.
using ImportVisibilityParams = ::testing::WithParamInterface<
std::tuple<ArgVector, std::tuple<const char *, const char *, bool>>>;
template <typename PatternFactory>
class ImportVisibility
: public ASTImporterTestBase,
public ImportVisibilityParams {
protected:
using DeclTy = typename PatternFactory::DeclTy;
ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
std::string getCode0() const { return std::get<0>(std::get<1>(GetParam())); }
std::string getCode1() const { return std::get<1>(std::get<1>(GetParam())); }
bool shouldBeLinked() const { return std::get<2>(std::get<1>(GetParam())); }
BindableMatcher<Decl> getPattern() const { return PatternFactory()(); }
void TypedTest_ImportAfter() {
TranslationUnitDecl *ToTu = getToTuDecl(getCode0(), Lang_CXX);
TranslationUnitDecl *FromTu = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
auto *ToF0 = FirstDeclMatcher<DeclTy>().match(ToTu, getPattern());
auto *FromF1 = FirstDeclMatcher<DeclTy>().match(FromTu, getPattern());
auto *ToF1 = Import(FromF1, Lang_CXX);
ASSERT_TRUE(ToF0);
ASSERT_TRUE(ToF1);
EXPECT_NE(ToF0, ToF1);
if (shouldBeLinked())
EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
else
EXPECT_FALSE(ToF1->getPreviousDecl());
}
void TypedTest_ImportAfterImport() {
TranslationUnitDecl *FromTu0 = getTuDecl(getCode0(), Lang_CXX, "input0.cc");
TranslationUnitDecl *FromTu1 = getTuDecl(getCode1(), Lang_CXX, "input1.cc");
auto *FromF0 =
FirstDeclMatcher<DeclTy>().match(FromTu0, getPattern());
auto *FromF1 =
FirstDeclMatcher<DeclTy>().match(FromTu1, getPattern());
auto *ToF0 = Import(FromF0, Lang_CXX);
auto *ToF1 = Import(FromF1, Lang_CXX);
ASSERT_TRUE(ToF0);
ASSERT_TRUE(ToF1);
EXPECT_NE(ToF0, ToF1);
if (shouldBeLinked())
EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
else
EXPECT_FALSE(ToF1->getPreviousDecl());
}
};
using ImportFunctionsVisibility = ImportVisibility<GetFunPattern>;
using ImportVariablesVisibility = ImportVisibility<GetVarPattern>;
// FunctionDecl.
TEST_P(ImportFunctionsVisibility, ImportAfter) {
TypedTest_ImportAfter();
}
TEST_P(ImportFunctionsVisibility, ImportAfterImport) {
TypedTest_ImportAfterImport();
}
// VarDecl.
TEST_P(ImportVariablesVisibility, ImportAfter) {
TypedTest_ImportAfter();
}
TEST_P(ImportVariablesVisibility, ImportAfterImport) {
TypedTest_ImportAfterImport();
}
bool ExpectLink = true;
bool ExpectNotLink = false;
INSTANTIATE_TEST_CASE_P(
ParameterizedTests, ImportFunctionsVisibility,
::testing::Combine(
DefaultTestValuesForRunOptions,
::testing::Values(std::make_tuple(ExternF, ExternF, ExpectLink),
std::make_tuple(ExternF, StaticF, ExpectNotLink),
std::make_tuple(ExternF, AnonF, ExpectNotLink),
std::make_tuple(StaticF, ExternF, ExpectNotLink),
std::make_tuple(StaticF, StaticF, ExpectNotLink),
std::make_tuple(StaticF, AnonF, ExpectNotLink),
std::make_tuple(AnonF, ExternF, ExpectNotLink),
std::make_tuple(AnonF, StaticF, ExpectNotLink),
std::make_tuple(AnonF, AnonF, ExpectNotLink))), );
INSTANTIATE_TEST_CASE_P(
ParameterizedTests, ImportVariablesVisibility,
::testing::Combine(
DefaultTestValuesForRunOptions,
::testing::Values(std::make_tuple(ExternV, ExternV, ExpectLink),
std::make_tuple(ExternV, StaticV, ExpectNotLink),
std::make_tuple(ExternV, AnonV, ExpectNotLink),
std::make_tuple(StaticV, ExternV, ExpectNotLink),
std::make_tuple(StaticV, StaticV, ExpectNotLink),
std::make_tuple(StaticV, AnonV, ExpectNotLink),
std::make_tuple(AnonV, ExternV, ExpectNotLink),
std::make_tuple(AnonV, StaticV, ExpectNotLink),
std::make_tuple(AnonV, AnonV, ExpectNotLink))), );
} // namespace TypeAndValueParameterizedTests
TEST_P(ASTImporterOptionSpecificTestBase, ImportVariableChainInC) {
std::string Code = "static int v; static int v = 0;";
auto Pattern = varDecl(hasName("v"));
TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_C, "input0.c");
auto *From0 = FirstDeclMatcher<VarDecl>().match(FromTu, Pattern);
auto *From1 = LastDeclMatcher<VarDecl>().match(FromTu, Pattern);
auto *To0 = Import(From0, Lang_C);
auto *To1 = Import(From1, Lang_C);
EXPECT_TRUE(To0);
ASSERT_TRUE(To1);
EXPECT_NE(To0, To1);
EXPECT_EQ(To1->getPreviousDecl(), To0);
}
TEST_P(ImportFunctions, ImportFromDifferentScopedAnonNamespace) {
TranslationUnitDecl *FromTu = getTuDecl(
"namespace NS0 { namespace { void f(); } }"
"namespace NS1 { namespace { void f(); } }",
Lang_CXX, "input0.cc");
auto Pattern = functionDecl(hasName("f"));
auto *FromF0 = FirstDeclMatcher<FunctionDecl>().match(FromTu, Pattern);
auto *FromF1 = LastDeclMatcher<FunctionDecl>().match(FromTu, Pattern);
auto *ToF0 = Import(FromF0, Lang_CXX);
auto *ToF1 = Import(FromF1, Lang_CXX);
EXPECT_TRUE(ToF0);
ASSERT_TRUE(ToF1);
EXPECT_NE(ToF0, ToF1);
EXPECT_FALSE(ToF1->getPreviousDecl());
}
TEST_P(ImportFunctions, ImportFunctionFromUnnamedNamespace) {
{
Decl *FromTU = getTuDecl("namespace { void f() {} } void g0() { f(); }",
Lang_CXX, "input0.cc");
auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
FromTU, functionDecl(hasName("g0")));
Import(FromD, Lang_CXX);
}
{
Decl *FromTU =
getTuDecl("namespace { void f() { int a; } } void g1() { f(); }",
Lang_CXX, "input1.cc");
auto *FromD = FirstDeclMatcher<FunctionDecl>().match(
FromTU, functionDecl(hasName("g1")));
Import(FromD, Lang_CXX);
}
Decl *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
ASSERT_EQ(DeclCounter<FunctionDecl>().match(ToTU, functionDecl(hasName("f"))),
2u);
}
struct ImportFriendFunctions : ImportFunctions {};
TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {