This patch addresses a problem encountered by the

ASTImporter when importing the following types:

typedef struct {
} A;

typedef struct {
  A a;
} B;

Suppose we have imported B, but we did not at that
time need to complete it.  Then later we want to
import A.  The struct is anonymous, so the first
thing we want to do is make sure no other anonymous
struct already matches it.  So we set up an
StructuralEquivalenceContext and compare B with A.

This happens at ASTImporter.cpp:2179.

Now, in this scenario, B is not complete.  So we go
and import its fields, including a, which causes A
to be imported.  The ASTImporter doesn’t yet have A
in its list of already-imported things, so we
import A.

After the StructuralEquivalenceContext is finished
determining that A and B are different, the
ASTImporter concludes that A must be imported
because no equivalent exists, so it imports a second
copy of A.  Now we have two different structs
representing A.  This is really bad news.

The patch allows the StructuralEquivalenceContext to
use the original version of B when making its
comparison, obviating the need for an import and
cutting this loop.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@192324 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/ASTImporter.h b/include/clang/AST/ASTImporter.h
index 1672ab22a3de2e5e057ecd7e6da3f62891053f76..b74c8ee1bf4713decdbfe57f621f6fa24d8491c4 100644 (file)
--- a/include/clang/AST/ASTImporter.h
+++ b/include/clang/AST/ASTImporter.h
@@ -271,6 +271,14 @@ namespace clang {
     /// Subclasses can override this function to observe all of the \c From ->
     /// \c To declaration mappings as they are imported.
     virtual Decl *Imported(Decl *From, Decl *To);
+      
+    /// \brief Called by StructuralEquivalenceContext.  If a RecordDecl is
+    /// being compared to another RecordDecl as part of import, completing the
+    /// other RecordDecl may trigger importation of the first RecordDecl. This
+    /// happens especially for anonymous structs.  If the original of the second
+    /// RecordDecl can be found, we can complete it without the need for
+    /// importation, eliminating this loop.
+    virtual Decl *GetOriginalDecl(Decl *To) { return NULL; }
     
     /// \brief Determine whether the given types are structurally
     /// equivalent.

diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index c6edc7712036655d59855954b3833ac5826ec7ed..4e49ba3db88edbaf3acbf7e904f168ac89e2ea28 100644 (file)
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -2178,8 +2178,17 @@ bool ASTNodeImporter::ImportTemplateArguments(const TemplateArgument *FromArgs,
 
 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) {
+    RecordDecl *ToOriginRecord = dyn_cast<RecordDecl>(ToOrigin);
+    if (ToOriginRecord)
+      ToRecord = ToOriginRecord;
+  }
+
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
-                                   Importer.getToContext(),
+                                   ToRecord->getASTContext(),
                                    Importer.getNonEquivalentDecls(),
                                    false, Complain);
   return Ctx.IsStructurallyEquivalent(FromRecord, ToRecord);