martong created this revision.
martong added a reviewer: a_sidorin.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.
Currently ASTImporter::NonEquivalentDecls member keeps its state and
shares it between consecutive calls of
StructuralEquivalenceContesxt::IsEquivalent(). Thus, we cache
inequivalent pairs from a previous snapshot of the AST. However, we
continuously build the AST and a pair which used to be inequivalent may
be equivalent later (or vica-versa). NonEquivalentDecls behaves
similarly to other internal states of StructuralEquivalenceContext
(TentativeEquivalences, DeclsToCheck). I.e this state too should be
reset before each IsEquivalent() call.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D62131
Files:
clang/include/clang/AST/ASTImporter.h
clang/include/clang/AST/ASTStructuralEquivalence.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/Sema/SemaType.cpp
clang/test/ASTMerge/class-template/test.cpp
clang/test/ASTMerge/enum/test.c
clang/test/ASTMerge/struct/test.c
clang/unittests/AST/StructuralEquivalenceTest.cpp
Index: clang/unittests/AST/StructuralEquivalenceTest.cpp
===================================================================
--- clang/unittests/AST/StructuralEquivalenceTest.cpp
+++ clang/unittests/AST/StructuralEquivalenceTest.cpp
@@ -78,14 +78,12 @@
}
bool testStructuralMatch(Decl *D0, Decl *D1) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls01;
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls10;
- StructuralEquivalenceContext Ctx01(
- D0->getASTContext(), D1->getASTContext(),
- NonEquivalentDecls01, StructuralEquivalenceKind::Default, false, false);
- StructuralEquivalenceContext Ctx10(
- D1->getASTContext(), D0->getASTContext(),
- NonEquivalentDecls10, StructuralEquivalenceKind::Default, false, false);
+ StructuralEquivalenceContext Ctx01(D0->getASTContext(), D1->getASTContext(),
+ StructuralEquivalenceKind::Default,
+ false, false);
+ StructuralEquivalenceContext Ctx10(D1->getASTContext(), D0->getASTContext(),
+ StructuralEquivalenceKind::Default,
+ false, false);
bool Eq01 = Ctx01.IsEquivalent(D0, D1);
bool Eq10 = Ctx10.IsEquivalent(D1, D0);
EXPECT_EQ(Eq01, Eq10);
Index: clang/test/ASTMerge/struct/test.c
===================================================================
--- clang/test/ASTMerge/struct/test.c
+++ clang/test/ASTMerge/struct/test.c
@@ -34,11 +34,6 @@
// CHECK: struct1.c:56:10: warning: type 'struct DeeperError' has incompatible definitions in different translation units
// CHECK: struct1.c:56:35: note: field 'f' has type 'int' here
// CHECK: struct2.c:53:37: note: field 'f' has type 'float' here
-// CHECK: struct1.c:54:8: warning: type 'struct DeepError' has incompatible definitions in different translation units
-// CHECK: struct1.c:56:41: note: field 'Deeper' has type 'struct DeeperError *' here
-// CHECK: struct2.c:53:43: note: field 'Deeper' has type 'struct DeeperError *' here
-// CHECK: struct2.c:54:3: warning: external variable 'xDeep' declared with incompatible types in different translation units ('struct DeepError' vs. 'struct DeepError')
-// CHECK: struct1.c:57:3: note: declared here with type 'struct DeepError'
// CHECK: struct1.c:74:9: warning: type 'S13' has incompatible definitions in different translation units
// CHECK: struct1.c:75:9: note: field 'i' has type 'Float' (aka 'float') here
// CHECK: struct2.c:72:7: note: field 'i' has type 'int' here
@@ -47,9 +42,5 @@
// CHECK: struct1.c:130:7: warning: type 'struct DeepUnnamedError::(anonymous at [[PATH_TO_INPUTS:.+]]struct1.c:130:7)' has incompatible definitions in different translation units
// CHECK: struct1.c:131:14: note: field 'i' has type 'long' here
// CHECK: struct2.c:128:15: note: field 'i' has type 'float' here
-// CHECK: struct1.c:129:5: warning: type 'union DeepUnnamedError::(anonymous at [[PATH_TO_INPUTS]]struct1.c:129:5)' has incompatible definitions in different translation units
-// CHECK: struct1.c:132:9: note: field 'S' has type 'struct (anonymous struct at [[PATH_TO_INPUTS]]struct1.c:130:7)' here
-// CHECK: struct2.c:129:9: note: field 'S' has type 'struct (anonymous struct at [[PATH_TO_INPUTS]]struct2.c:127:7)' here
// CHECK: struct2.c:138:3: warning: external variable 'x16' declared with incompatible types in different translation units ('struct DeepUnnamedError' vs. 'struct DeepUnnamedError')
// CHECK: struct1.c:141:3: note: declared here with type 'struct DeepUnnamedError'
-// CHECK: 20 warnings generated
Index: clang/test/ASTMerge/enum/test.c
===================================================================
--- clang/test/ASTMerge/enum/test.c
+++ clang/test/ASTMerge/enum/test.c
@@ -22,4 +22,3 @@
// CHECK: enum1.c:30:6: note: no corresponding enumerator here
// CHECK: enum2.c:34:3: warning: external variable 'x5' declared with incompatible types in different translation units ('enum E5' vs. 'enum E5')
// CHECK: enum1.c:34:3: note: declared here with type 'enum E5'
-// CHECK: 8 warnings generated
Index: clang/test/ASTMerge/class-template/test.cpp
===================================================================
--- clang/test/ASTMerge/class-template/test.cpp
+++ clang/test/ASTMerge/class-template/test.cpp
@@ -24,5 +24,4 @@
// CHECK: class-template1.cpp:36:7: note: field 'member' has type 'int' here
// CHECK: class-template2.cpp:36:9: note: field 'member' has type 'float' here
-// CHECK: 6 warnings generated.
// CHECK-NOT: static_assert
Index: clang/lib/Sema/SemaType.cpp
===================================================================
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7740,17 +7740,15 @@
}
bool Sema::hasStructuralCompatLayout(Decl *D, Decl *Suggested) {
- llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
if (!Suggested)
return false;
// FIXME: Add a specific mode for C11 6.2.7/1 in StructuralEquivalenceContext
// and isolate from other C++ specific checks.
StructuralEquivalenceContext Ctx(
- D->getASTContext(), Suggested->getASTContext(), NonEquivalentDecls,
- StructuralEquivalenceKind::Default,
- false /*StrictTypeSpelling*/, true /*Complain*/,
- true /*ErrorOnTagTypeMismatch*/);
+ D->getASTContext(), Suggested->getASTContext(),
+ StructuralEquivalenceKind::Default, false /*StrictTypeSpelling*/,
+ true /*Complain*/, true /*ErrorOnTagTypeMismatch*/);
return Ctx.IsEquivalent(D, Suggested);
}
Index: clang/lib/AST/ASTStructuralEquivalence.cpp
===================================================================
--- clang/lib/AST/ASTStructuralEquivalence.cpp
+++ clang/lib/AST/ASTStructuralEquivalence.cpp
@@ -1603,6 +1603,7 @@
// as a side effect of one inequivalent element in the DeclsToCheck list.
assert(DeclsToCheck.empty());
assert(TentativeEquivalences.empty());
+ assert(NonEquivalentDecls.empty());
if (!::IsStructurallyEquivalent(*this, D1, D2))
return false;
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1924,8 +1924,7 @@
bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
- false, Complain);
+ getStructuralEquivalenceKind(Importer), false, Complain);
return Ctx.IsEquivalent(From, To);
}
@@ -1942,7 +1941,6 @@
StructuralEquivalenceContext Ctx(Importer.getFromContext(),
ToRecord->getASTContext(),
- Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer),
false, Complain);
return Ctx.IsEquivalent(FromRecord, ToRecord);
@@ -1952,8 +1950,7 @@
bool Complain) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
- false, Complain);
+ getStructuralEquivalenceKind(Importer), false, Complain);
return Ctx.IsEquivalent(FromVar, ToVar);
}
@@ -1964,9 +1961,9 @@
if (auto *ToOriginEnum = dyn_cast<EnumDecl>(ToOrigin))
ToEnum = ToOriginEnum;
- StructuralEquivalenceContext Ctx(
- Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
+ StructuralEquivalenceContext Ctx(Importer.getFromContext(),
+ Importer.getToContext(),
+ getStructuralEquivalenceKind(Importer));
return Ctx.IsEquivalent(FromEnum, ToEnum);
}
@@ -1974,16 +1971,14 @@
FunctionTemplateDecl *To) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
- false, false);
+ getStructuralEquivalenceKind(Importer), false, false);
return Ctx.IsEquivalent(From, To);
}
bool ASTNodeImporter::IsStructuralMatch(FunctionDecl *From, FunctionDecl *To) {
StructuralEquivalenceContext Ctx(
Importer.getFromContext(), Importer.getToContext(),
- Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
- false, false);
+ getStructuralEquivalenceKind(Importer), false, false);
return Ctx.IsEquivalent(From, To);
}
@@ -2001,7 +1996,6 @@
ClassTemplateDecl *To) {
StructuralEquivalenceContext Ctx(Importer.getFromContext(),
Importer.getToContext(),
- Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer));
return Ctx.IsEquivalent(From, To);
}
@@ -2010,7 +2004,6 @@
VarTemplateDecl *To) {
StructuralEquivalenceContext Ctx(Importer.getFromContext(),
Importer.getToContext(),
- Importer.getNonEquivalentDecls(),
getStructuralEquivalenceKind(Importer));
return Ctx.IsEquivalent(From, To);
}
@@ -8549,7 +8542,7 @@
}
}
- StructuralEquivalenceContext Ctx(FromContext, ToContext, NonEquivalentDecls,
+ StructuralEquivalenceContext Ctx(FromContext, ToContext,
getStructuralEquivalenceKind(*this), false,
Complain);
return Ctx.IsEquivalent(From, To);
Index: clang/include/clang/AST/ASTStructuralEquivalence.h
===================================================================
--- clang/include/clang/AST/ASTStructuralEquivalence.h
+++ clang/include/clang/AST/ASTStructuralEquivalence.h
@@ -53,7 +53,7 @@
/// Declaration (from, to) pairs that are known not to be equivalent
/// (which we have already complained about).
- llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls;
+ llvm::DenseSet<std::pair<Decl *, Decl *>> NonEquivalentDecls;
StructuralEquivalenceKind EqKind;
@@ -70,14 +70,13 @@
/// \c true if the last diagnostic came from ToCtx.
bool LastDiagFromC2 = false;
- StructuralEquivalenceContext(
- ASTContext &FromCtx, ASTContext &ToCtx,
- llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls,
- StructuralEquivalenceKind EqKind,
- bool StrictTypeSpelling = false, bool Complain = true,
- bool ErrorOnTagTypeMismatch = false)
- : FromCtx(FromCtx), ToCtx(ToCtx), NonEquivalentDecls(NonEquivalentDecls),
- EqKind(EqKind), StrictTypeSpelling(StrictTypeSpelling),
+ StructuralEquivalenceContext(ASTContext &FromCtx, ASTContext &ToCtx,
+ StructuralEquivalenceKind EqKind,
+ bool StrictTypeSpelling = false,
+ bool Complain = true,
+ bool ErrorOnTagTypeMismatch = false)
+ : FromCtx(FromCtx), ToCtx(ToCtx), EqKind(EqKind),
+ StrictTypeSpelling(StrictTypeSpelling),
ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain) {}
DiagnosticBuilder Diag1(SourceLocation Loc, unsigned DiagID);
Index: clang/include/clang/AST/ASTImporter.h
===================================================================
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -83,7 +83,6 @@
class ASTImporter {
friend class ASTNodeImporter;
public:
- using NonEquivalentDeclSet = llvm::DenseSet<std::pair<Decl *, Decl *>>;
using ImportedCXXBaseSpecifierMap =
llvm::DenseMap<const CXXBaseSpecifier *, CXXBaseSpecifier *>;
@@ -133,10 +132,6 @@
/// in the "to" source manager.
ImportedCXXBaseSpecifierMap ImportedCXXBaseSpecifiers;
- /// Declaration (from, to) pairs that are known not to be equivalent
- /// (which we have already complained about).
- NonEquivalentDeclSet NonEquivalentDecls;
-
using FoundDeclsTy = SmallVector<NamedDecl *, 2>;
FoundDeclsTy findDeclsInToCtx(DeclContext *DC, DeclarationName Name);
@@ -378,9 +373,6 @@
/// Report a diagnostic in the "from" context.
DiagnosticBuilder FromDiag(SourceLocation Loc, unsigned DiagID);
- /// Return the set of declarations that we know are not equivalent.
- NonEquivalentDeclSet &getNonEquivalentDecls() { return NonEquivalentDecls; }
-
/// Called for ObjCInterfaceDecl, ObjCProtocolDecl, and TagDecl.
/// Mark the Decl as complete, filling it in as much as possible.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
