martong created this revision.
martong added reviewers: a.sidorin, a_sidorin, r.stahl.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.
Implementation functions call into the member functions of
ASTStructuralEquivalence, thus they can falsely alter the DeclsToCheck state
(they add decls). This results that some leaf declarations can be stated as
inequivalent as a side effect of one inequivalent element in the DeclsToCheck
list. And since we store the non-equivalencies, any (otherwise independent)
decls will be rendered as non-equivalent. Solution: I tried to clearly
separate the implementation functions (the static ones) and the public
interface. From now on, the implementation functions do not call any public
member functions, only other implementation functions.
Repository:
rC Clang
https://reviews.llvm.org/D49300
Files:
include/clang/AST/ASTStructuralEquivalence.h
lib/AST/ASTImporter.cpp
lib/AST/ASTStructuralEquivalence.cpp
lib/Sema/SemaType.cpp
unittests/AST/StructuralEquivalenceTest.cpp
Index: unittests/AST/StructuralEquivalenceTest.cpp
===================================================================
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -67,7 +67,7 @@
StructuralEquivalenceContext Ctx(
D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls,
StructuralEquivalenceKind::Default, false, false);
- return Ctx.IsStructurallyEquivalent(D0, D1);
+ return Ctx.IsEquivalent(D0, D1);
}
bool testStructuralMatch(std::tuple<NamedDecl *, NamedDecl *> t) {
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7540,7 +7540,7 @@
StructuralEquivalenceKind::Default,
false /*StrictTypeSpelling*/, true /*Complain*/,
true /*ErrorOnTagTypeMismatch*/);
- return Ctx.IsStructurallyEquivalent(D, Suggested);
+ return Ctx.IsEquivalent(D, Suggested);
}
/// Determine whether there is any declaration of \p D that was ever a
Index: lib/AST/ASTStructuralEquivalence.cpp
===================================================================
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -184,18 +184,18 @@
return true;
case TemplateArgument::Type:
- return Context.IsStructurallyEquivalent(Arg1.getAsType(), Arg2.getAsType());
+ return IsStructurallyEquivalent(Context, Arg1.getAsType(), Arg2.getAsType());
case TemplateArgument::Integral:
- if (!Context.IsStructurallyEquivalent(Arg1.getIntegralType(),
+ if (!IsStructurallyEquivalent(Context, Arg1.getIntegralType(),
Arg2.getIntegralType()))
return false;
return llvm::APSInt::isSameValue(Arg1.getAsIntegral(),
Arg2.getAsIntegral());
case TemplateArgument::Declaration:
- return Context.IsStructurallyEquivalent(Arg1.getAsDecl(), Arg2.getAsDecl());
+ return IsStructurallyEquivalent(Context, Arg1.getAsDecl(), Arg2.getAsDecl());
case TemplateArgument::NullPtr:
return true; // FIXME: Is this correct?
@@ -1191,8 +1191,8 @@
return false;
}
- if (!Context.IsStructurallyEquivalent(Params1->getParam(I),
- Params2->getParam(I)))
+ if (!IsStructurallyEquivalent(Context, Params1->getParam(I),
+ Params2->getParam(I)))
return false;
}
@@ -1229,7 +1229,7 @@
}
// Check types.
- if (!Context.IsStructurallyEquivalent(D1->getType(), D2->getType())) {
+ if (!IsStructurallyEquivalent(Context, D1->getType(), D2->getType())) {
if (Context.Complain) {
Context.Diag2(D2->getLocation(),
diag::err_odr_non_type_parameter_type_inconsistent)
@@ -1280,8 +1280,8 @@
return false;
// Check the templated declaration.
- return Context.IsStructurallyEquivalent(D1->getTemplatedDecl(),
- D2->getTemplatedDecl());
+ return IsStructurallyEquivalent(Context, D1->getTemplatedDecl(),
+ D2->getTemplatedDecl());
}
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
@@ -1292,8 +1292,8 @@
return false;
// Check the templated declaration.
- return Context.IsStructurallyEquivalent(D1->getTemplatedDecl()->getType(),
- D2->getTemplatedDecl()->getType());
+ return IsStructurallyEquivalent(Context, D1->getTemplatedDecl()->getType(),
+ D2->getTemplatedDecl()->getType());
}
static bool IsStructurallyEquivalent(StructuralEquivalenceContext &Context,
@@ -1404,16 +1404,26 @@
return Index;
}
-bool StructuralEquivalenceContext::IsStructurallyEquivalent(Decl *D1,
- Decl *D2) {
+bool StructuralEquivalenceContext::IsEquivalent(Decl *D1, Decl *D2) {
+
+ // Ensure that the implementation functions (all static functions in this TU)
+ // never call the public ASTStructuralEquivalence::IsEquivalent() functions,
+ // because that will wreak havoc the internal state (DeclsToCheck and
+ // TentativeEquivalences members) and can cause faulty behaviour. For
+ // instance, some leaf declarations can be stated and cached as inequivalent
+ // as a side effect of one inequivalent element in the DeclsToCheck list.
+ assert(DeclsToCheck.empty());
+ assert(TentativeEquivalences.empty());
+
if (!::IsStructurallyEquivalent(*this, D1, D2))
return false;
return !Finish();
}
-bool StructuralEquivalenceContext::IsStructurallyEquivalent(QualType T1,
- QualType T2) {
+bool StructuralEquivalenceContext::IsEquivalent(QualType T1, QualType T2) {
+ assert(DeclsToCheck.empty());
+ assert(TentativeEquivalences.empty());
if (!::IsStructurallyEquivalent(*this, T1, T2))
return false;
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -294,6 +294,7 @@
bool ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
+ bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
@@ -1581,7 +1582,15 @@
: StructuralEquivalenceKind::Default;
}
-bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
+bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain) {
+ StructuralEquivalenceContext Ctx(
+ Importer.getFromContext(), Importer.getToContext(),
+ Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
+ false, Complain);
+ return Ctx.IsEquivalent(From, To);
+}
+
+bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
RecordDecl *ToRecord, bool Complain) {
// Eliminate a potential failure point where we attempt to re-import
// something we're trying to import while completing ToRecord.
@@ -1597,40 +1606,40 @@
Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer),
false, Complain);
- return Ctx.IsStructurallyEquivalent(FromRecord, ToRecord);
+ return Ctx.IsEquivalent(FromRecord, ToRecord);
}
bool ASTNodeImporter::IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
bool Complain) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
false, Complain);
- return Ctx.IsStructurallyEquivalent(FromVar, ToVar);
+ return Ctx.IsEquivalent(FromVar, ToVar);
}
bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
- return Ctx.IsStructurallyEquivalent(FromEnum, ToEnum);
+ return Ctx.IsEquivalent(FromEnum, ToEnum);
}
bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
FunctionTemplateDecl *To) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
false, false);
- return Ctx.IsStructurallyEquivalent(From, To);
+ return Ctx.IsEquivalent(From, To);
}
bool ASTNodeImporter::IsStructuralMatch(FunctionDecl *From, FunctionDecl *To) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
false, false);
- return Ctx.IsStructurallyEquivalent(From, To);
+ return Ctx.IsEquivalent(From, To);
}
bool ASTNodeImporter::IsStructuralMatch(EnumConstantDecl *FromEC,
@@ -1649,16 +1658,16 @@
Importer.getToContext(),
Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer));
- return Ctx.IsStructurallyEquivalent(From, To);
+ return Ctx.IsEquivalent(From, To);
}
bool ASTNodeImporter::IsStructuralMatch(VarTemplateDecl *From,
VarTemplateDecl *To) {
StructuralEquivalenceContext Ctx(Importer.getFromContext(),
Importer.getToContext(),
Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer));
- return Ctx.IsStructurallyEquivalent(From, To);
+ return Ctx.IsEquivalent(From, To);
}
Decl *ASTNodeImporter::VisitDecl(Decl *D) {
@@ -2942,15 +2951,11 @@
// FriendDecl is not a NamedDecl so we cannot use localUncachedLookup.
auto *RD = cast<CXXRecordDecl>(DC);
FriendDecl *ImportedFriend = RD->getFirstFriend();
- StructuralEquivalenceContext Context(
- Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
- false, false);
while (ImportedFriend) {
if (D->getFriendDecl() && ImportedFriend->getFriendDecl()) {
- if (Context.IsStructurallyEquivalent(D->getFriendDecl(),
- ImportedFriend->getFriendDecl()))
+ if (IsStructuralMatch(D->getFriendDecl(), ImportedFriend->getFriendDecl(),
+ /*Complain=*/false))
return Importer.MapImported(D, ImportedFriend);
} else if (D->getFriendType() && ImportedFriend->getFriendType()) {
@@ -7585,5 +7590,5 @@
StructuralEquivalenceContext Ctx(FromContext, ToContext, NonEquivalentDecls,
getStructuralEquivalenceKind(*this), false,
Complain);
- return Ctx.IsStructurallyEquivalent(From, To);
+ return Ctx.IsEquivalent(From, To);
}
Index: include/clang/AST/ASTStructuralEquivalence.h
===================================================================
--- include/clang/AST/ASTStructuralEquivalence.h
+++ include/clang/AST/ASTStructuralEquivalence.h
@@ -85,10 +85,18 @@
/// Determine whether the two declarations are structurally
/// equivalent.
- bool IsStructurallyEquivalent(Decl *D1, Decl *D2);
+ /// Implementation functions (all static functions in
+ /// ASTStructuralEquivalence.cpp) must never call this function because that
+ /// will wreak havoc the internal state (\c DeclsToCheck and
+ /// \c TentativeEquivalences members) and can cause faulty equivalent results.
+ bool IsEquivalent(Decl *D1, Decl *D2);
/// Determine whether the two types are structurally equivalent.
- bool IsStructurallyEquivalent(QualType T1, QualType T2);
+ /// Implementation functions (all static functions in
+ /// ASTStructuralEquivalence.cpp) must never call this function because that
+ /// will wreak havoc the internal state (\c DeclsToCheck and
+ /// \c TentativeEquivalences members) and can cause faulty equivalent results.
+ bool IsEquivalent(QualType T1, QualType T2);
/// Find the index of the given anonymous struct/union within its
/// context.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
