[ASTImporter] Remove NonEquivalentDecls from ASTImporter

[martong]

Summary

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.

[a_sidorin]

Hi Gabor,
Could you provide an example of an import sequence leading to this behavior? It's hard for me to imagine such a situation.

clang/test/ASTMerge/struct/test.c

37

It looks strange to me that these warnings are gone.

[martong]

In D62131#1517634, @a_sidorin wrote:

Hi Gabor,
Could you provide an example of an import sequence leading to this behavior? It's hard for me to imagine such a situation.

Alexei, thanks for the review again. I don't think I could create an import sequence for this. However, I have experienced this kind of poisonous cache behavior during the debugging of a mysterious false positive structural inequivalency (during the analysis of protobuf).
The following happened: During the analysis we compared two Decls which turned out to be inequivalent, so we cached them. Later during the analysis, however, we added a new node to the redecl chain of one of these Decls which we previously compared. Then another structural equivalent check followed for the two Decls. And this time they should have been considered structurally equivalent, but the cache already contained them as nonequivalent. This resulted in a false positive NameConflict error.

By this change the error had gone, and we did not recognize any analysis slowdown. Remember, we still have a cache, but not a global cache in the ASTImporter object.

clang/test/ASTMerge/struct/test.c

37

For me, these warnings seems just like noise, by themselves line 37-40 does not show where is the exact mismatch.
The exact mismatch is shown in line 34-36, those warnings indicate that DeeperError has incompatible fields in the different TUs with different types (int vs float). That is the lowest level where the mismatch happens, I don't think we should indicate a warning for all other types which contain the mismatched types.

The same is true below (line 50-52) with the struct and union.

[a_sidorin]

The following happened: During the analysis we compared two Decls which turned out to be inequivalent, so we cached them. Later during the analysis, however, we added a new node to the redecl chain of one of these Decls which we previously compared. Then another structural equivalent check followed for the two Decls. And this time they should have been considered structurally equivalent, but the cache already contained them as nonequivalent. This resulted in a false positive NameConflict error.

Should we reset the non-equivalence relation after a decl is imported for this decl and its redecls?

By this change the error had gone, and we did not recognize any analysis slowdown. Remember, we still have a cache, but not a global cache in the ASTImporter object.

Hm, I wonder if our cache is really useful or not. Unfortunately, I never did any measures.

Still the lack of a test disturbs me, even despite the fact that I got the idea.

[balazske]

Example about how to get wrong things into NonEquivalentDecls:
We want to compare class C for structural equivalence in the following codes:

class A {}; class B {int i;}; void x(A *); void y(A *); class C { friend void x(A *); friend void y(A *); };

and

class A {}; class B {int i;}; void x(A *); void y(B *); class C { friend void x(A *); friend void y(B *); };

The result is false for C but the NonEquivalentDecls will contain after the compare a void x(A*)<->void x(A*) pair.

The reason is that during the check we encounter first a A<->B pair (iterating over the friends and friend functions, first y is encountered and a check for A and B follows). The B is recorded as "tentative equivalence" to A. Then we try to check A to A (during check of x) but because there is a "tentative equivalence" with B from A the check returns false (not equivalent). This false result is recorded as a (incorrect) non-equivalence of the two x functions.
I want to replace the DeclsToCheck and TentativeEquivalences with a set of Decl pairs (like the NonEquivalentDecls) so it will be possible to record the same from Decl with multiple to values. (Is there a reason for why there is a NonEquivalentDecls cache but not a EquivalentDecls or simply a cache for result values?)

[balazske]

A "quick fix" for the problem:
https://reviews.llvm.org/D66538

[martong]

In D62131#1639168, @balazske wrote:

Example about how to get wrong things into NonEquivalentDecls:
We want to compare class C for structural equivalence in the following codes:

class A {}; class B {int i;}; void x(A *); void y(A *); class C { friend void x(A *); friend void y(A *); };

and

class A {}; class B {int i;}; void x(A *); void y(B *); class C { friend void x(A *); friend void y(B *); };

The result is false for C but the NonEquivalentDecls will contain after the compare a void x(A*)<->void x(A*) pair.

The reason is that during the check we encounter first a A<->B pair (iterating over the friends and friend functions, first y is encountered and a check for A and B follows). The B is recorded as "tentative equivalence" to A. Then we try to check A to A (during check of x) but because there is a "tentative equivalence" with B from A the check returns false (not equivalent). This false result is recorded as a (incorrect) non-equivalence of the two x functions.
I want to replace the DeclsToCheck and TentativeEquivalences with a set of Decl pairs (like the NonEquivalentDecls) so it will be possible to record the same from Decl with multiple to values. (Is there a reason for why there is a NonEquivalentDecls cache but not a EquivalentDecls or simply a cache for result values?)

Yes, this is indeed a problem. But it is independent from the original problem this patch intends to solve.

[martong]

@a_sidorin Alexei,

With Balazs we had a long discussion and I think we reached a consensus and common understanding. Now I try to summarize it.

It turned out that this patch essentially renders the NonEquivalentDecls cache to be completely disabled, i.e. we never have a cache hit. (I tried an example test run with an assertion enabled for the cache hit.)
This is because in Finish(), when we find the first non-equivalent pair then we immediately break out from the while loop which pops out elements from the queue. This means we do not consider any elements left in the queue after this, so the "local" cache is never going to be used again.
This, however, explains why did we have better results with this patch: the cache was disabled, so those faulty entries which were cached (see Balazs's example above) could not cause a faulty behavior.

Secondly, I tried to fabricate a test case where the change of the redecl chain could render previously non-equivalent pairs to be equivalent. I could not create such. I also executed an experiment on llvm/master to see in case of a cache hit whether we would get a different result without the cache:

   // Check whether we already know that these two declarations are not
   // structurally equivalent.
   if (Context.NonEquivalentDecls.count(
-          std::make_pair(D1->getCanonicalDecl(), D2->getCanonicalDecl())))
+          std::make_pair(D1->getCanonicalDecl(), D2->getCanonicalDecl()))) {
+    llvm::errs() << "Cache hit\n";
+    llvm::DenseSet<std::pair<Decl *, Decl *>> LocalNeqDecls;
+    StructuralEquivalenceContext LocalCtx(Context.FromCtx, Context.ToCtx,
+                                     LocalNeqDecls, Context.EqKind, false,
+                                     false, false);
+    assert(!LocalCtx.IsEquivalent(D1, D2));
     return false;
+  }

The added assertion did not fire. (I executed it on Xerces for a ~100 TUs). So, it is highly probable that if some pairs were ever non-equivalent they will stay non-equivalent.
This is not true, however, the other way around. Equivalent decls may become non-equivalent (e.g. a fwd decl is being completed).

To solve the issue that is found by Balazs, I suggest to move on with https://reviews.llvm.org/D66538 and abandon this patch.
An alternative solution would be to cache after the Finish() has returned, but I prefer D66538 because it makes the whole algorithm simpler, plus we can get rid of the redecleration chain check. By getting rid of that, I believe we can have a more robust import mechanism, which is more resistant to lookup errors. ATM, if we have a lookup error then we end up having two identical declarations in different redecl chains, which may initiate a NameConflict error avalanche. With D66538 we can avoid that.

As a next step, probably we should remove the cache because it complicates the code and the added performance value is not recognizable. If you'd like I can do measurements to investigate the performance difference. However, on our internal fork we disabled the cache entirely for quite a long time (this patch is originated from there) and we did not experience any noticeable performance degradation during the analysis.

[martong]

... it is highly probable that if some pairs were ever non-equivalent they will stay non-equivalent.

This may not be true in case of LLDB, because usually they do a minimal import, and in a later phase they do a normal import which imports e.g. the members of a class.
Clearly, in this case it is wrong to cache non-equivalent decls.

clang/include/clang/AST/ASTImporter.h

class TypeSourceInfo;
   // the different entries in a given redecl chain.
   llvm::SmallVector<Decl*, 2> getCanonicalForwardRedeclChain(Decl* D);
 
   /// Imports selected nodes from one AST context into another context,
   /// merging AST nodes where appropriate.
   class ASTImporter {
     friend class ASTNodeImporter;
   public:
-    using NonEquivalentDeclSet = llvm::DenseSet<std::pair<Decl *, Decl *>>;
     using ImportedCXXBaseSpecifierMap =
         llvm::DenseMap<const CXXBaseSpecifier *, CXXBaseSpecifier *>;
 
   private:
 
     /// Pointer to the import specific lookup table, which may be shared
     /// amongst several ASTImporter objects.
     /// This is an externally managed resource (and should exist during the
private:
     /// manager to the corresponding FileIDs in the "to" source manager.
     llvm::DenseMap<FileID, FileID> ImportedFileIDs;
 
     /// Mapping from the already-imported CXXBasesSpecifier in
     ///  the "from" source manager to the corresponding CXXBasesSpecifier
     ///  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);
 
     void AddToLookupTable(Decl *ToD);
 
   protected:
     /// Can be overwritten by subclasses to implement their own import logic.
     /// The overwritten method should call this method if it didn't import the
public:
     FileManager &getFromFileManager() const { return FromFileManager; }
 
     /// Report a diagnostic in the "to" context.
     DiagnosticBuilder ToDiag(SourceLocation Loc, unsigned DiagID);
 
     /// 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.
     ///
     /// \param D A declaration in the "to" context.
     virtual void CompleteDecl(Decl* D);
 
     /// Subclasses can override this function to observe all of the \c From ->
     /// \c To declaration mappings as they are imported.

clang/include/clang/AST/ASTStructuralEquivalence.h

struct StructuralEquivalenceContext {
   llvm::DenseMap<Decl *, Decl *> TentativeEquivalences;
 
   /// Queue of declarations in the first context whose equivalence
   /// with a declaration in the second context still needs to be verified.
   std::deque<Decl *> DeclsToCheck;
 
   /// 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;
 
   /// Whether we're being strict about the spelling of types when
   /// unifying two types.
   bool StrictTypeSpelling;
 
   /// Whether warn or error on tag type mismatches.
   bool ErrorOnTagTypeMismatch;
 
   /// Whether to complain about failures.
   bool Complain;
 
   /// \c true if the last diagnostic came from ToCtx.
   bool LastDiagFromC2 = false;
 
-  StructuralEquivalenceContext(
-      ASTContext &FromCtx, ASTContext &ToCtx,
-      llvm::DenseSet<std::pair<Decl *, Decl *>> &NonEquivalentDecls,
+  StructuralEquivalenceContext(ASTContext &FromCtx, ASTContext &ToCtx,
                                StructuralEquivalenceKind EqKind,
-      bool StrictTypeSpelling = false, bool Complain = true,
+                               bool StrictTypeSpelling = false,
+                               bool Complain = true,
                                bool ErrorOnTagTypeMismatch = false)
-      : FromCtx(FromCtx), ToCtx(ToCtx), NonEquivalentDecls(NonEquivalentDecls),
-        EqKind(EqKind), StrictTypeSpelling(StrictTypeSpelling),
+      : FromCtx(FromCtx), ToCtx(ToCtx), EqKind(EqKind),
+        StrictTypeSpelling(StrictTypeSpelling),
         ErrorOnTagTypeMismatch(ErrorOnTagTypeMismatch), Complain(Complain) {}
 
   DiagnosticBuilder Diag1(SourceLocation Loc, unsigned DiagID);
   DiagnosticBuilder Diag2(SourceLocation Loc, unsigned DiagID);
 
   /// Determine whether the two declarations are structurally
   /// equivalent.
   /// Implementation functions (all static functions in

clang/lib/AST/ASTImporter.cpp

 getStructuralEquivalenceKind(const ASTImporter &Importer) {
   return Importer.isMinimalImport() ? StructuralEquivalenceKind::Minimal
                                     : StructuralEquivalenceKind::Default;
 }
 
 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);
 }
 
 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.
   Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
   if (ToOrigin) {
     auto *ToOriginRecord = dyn_cast<RecordDecl>(ToOrigin);
     if (ToOriginRecord)
       ToRecord = ToOriginRecord;
   }
 
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
                                    ToRecord->getASTContext(),
-                                   Importer.getNonEquivalentDecls(),
                                    getStructuralEquivalenceKind(Importer),
                                    false, Complain);
   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);
+      getStructuralEquivalenceKind(Importer), false, Complain);
   return Ctx.IsEquivalent(FromVar, ToVar);
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToEnum.
   if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
     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);
 }
 
 bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
                                         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);
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumConstantDecl *FromEC,
                                         EnumConstantDecl *ToEC) {
   const llvm::APSInt &FromVal = FromEC->getInitVal();
   const llvm::APSInt &ToVal = ToEC->getInitVal();
 
   return FromVal.isSigned() == ToVal.isSigned() &&
          FromVal.getBitWidth() == ToVal.getBitWidth() &&
          FromVal == ToVal;
 }
 
 bool ASTNodeImporter::IsStructuralMatch(ClassTemplateDecl *From,
                                         ClassTemplateDecl *To) {
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
                                    Importer.getToContext(),
-                                   Importer.getNonEquivalentDecls(),
                                    getStructuralEquivalenceKind(Importer));
   return Ctx.IsEquivalent(From, To);
 }
 
 bool ASTNodeImporter::IsStructuralMatch(VarTemplateDecl *From,
                                         VarTemplateDecl *To) {
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
                                    Importer.getToContext(),
-                                   Importer.getNonEquivalentDecls(),
                                    getStructuralEquivalenceKind(Importer));
   return Ctx.IsEquivalent(From, To);
 }
 
 ExpectedDecl ASTNodeImporter::VisitDecl(Decl *D) {
   Importer.FromDiag(D->getLocation(), diag::err_unsupported_ast_node)
     << D->getDeclKindName();
   return make_error<ImportError>(ImportError::UnsupportedConstruct);
if (Pos != ImportedTypes.end()) {
     if (ExpectedType ToFromOrErr = Import(From)) {
       if (ToContext.hasSameType(*ToFromOrErr, To))
         return true;
     } else {
       llvm::consumeError(ToFromOrErr.takeError());
     }
   }
 
-  StructuralEquivalenceContext Ctx(FromContext, ToContext, NonEquivalentDecls,
+  StructuralEquivalenceContext Ctx(FromContext, ToContext,
                                    getStructuralEquivalenceKind(*this), false,
                                    Complain);
   return Ctx.IsEquivalent(From, To);
 }

clang/lib/AST/ASTStructuralEquivalence.cpp

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());
+  assert(NonEquivalentDecls.empty());
 
   if (!::IsStructurallyEquivalent(*this, D1, D2))
     return false;
 
   return !Finish();
 }
 
 bool StructuralEquivalenceContext::IsEquivalent(QualType T1, QualType T2) {

clang/lib/Sema/SemaType.cpp

if (!Tag->getDecl()->isCompleteDefinitionRequired()) {
       Tag->getDecl()->setCompleteDefinitionRequired();
       Consumer.HandleTagDeclRequiredDefinition(Tag->getDecl());
     }
   }
   return false;
 }
 
 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);
 }
 
 /// Determine whether there is any declaration of \p D that was ever a
 ///        definition (perhaps before module merging) and is currently visible.
 /// \param D The definition of the entity.
 /// \param Suggested Filled in with the declaration that should be made visible
 ///        in order to provide a definition of this entity.

clang/test/ASTMerge/class-template/test.cpp

 
 // CHECK: class-template2.cpp:27:20: warning: external variable 'x0r' declared with incompatible types in different translation units ('X0<double> *' vs. 'X0<float> *')
 // CHECK: class-template1.cpp:26:19: note: declared here with type 'X0<float> *'
 
 // CHECK: class-template1.cpp:35:8: warning: type 'X0<wchar_t>' has incompatible definitions in different translation units
 // 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

clang/test/ASTMerge/enum/test.c

 // CHECK: enum2.c:23:6: note: no corresponding enumerator here
 // CHECK: enum2.c:26:3: warning: external variable 'x4' declared with incompatible types in different translation units ('enum E4' vs. 'enum E4')
 // CHECK: enum1.c:27:3: note: declared here with type 'enum E4'
 // CHECK: enum1.c:30:6: warning: type 'enum E5' has incompatible definitions in different translation units
 // CHECK: enum2.c:33:3: note: enumerator 'E5Enumerator4' with value 3 here
 // 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

clang/test/ASTMerge/struct/test.c

 // CHECK: struct1.c:36:8: warning: type 'struct S7' has incompatible definitions in different translation units
 // CHECK: struct1.c:36:33: note: bit-field 'j' with type 'unsigned int' and length 8 here
 // CHECK: struct2.c:33:33: note: bit-field 'j' with type 'unsigned int' and length 16 here
 // CHECK: struct2.c:33:43: warning: external variable 'x7' declared with incompatible types in different translation units ('struct S7' vs. 'struct S7')
 // CHECK: struct1.c:36:42: note: declared here with type 'struct S7'
 // 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
 // CHECK: struct2.c:76:5: warning: external variable 'x13' declared with incompatible types in different translation units ('S13' vs. 'S13')
 // CHECK: struct1.c:79:5: note: declared here with type 'S13'
 // 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

clang/unittests/AST/StructuralEquivalenceTest.cpp

struct StructuralEquivalenceTest : ::testing::Test {
   std::tuple<NamedDecl *, NamedDecl *> makeNamedDecls(
       const std::string &SrcCode0, const std::string &SrcCode1,
       Language Lang, const char *const Identifier = "foo") {
     auto Matcher = namedDecl(hasName(Identifier));
     return makeDecls<NamedDecl>(SrcCode0, SrcCode1, Lang, Matcher);
   }
 
   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);
     return Eq01;
   }
 
   bool testStructuralMatch(std::tuple<Decl *, Decl *> t) {
     return testStructuralMatch(get<0>(t), get<1>(t));