[PATCH] D44100: [ASTImporter] Reorder fields after structure import is finished

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin created this revision.
a.sidorin added reviewers: xazax.hun, szepet, jingham.
Herald added subscribers: martong, rnkovacs.

- There are multiple reasons why field structures can be imported in wrong way. The simplest is the ability of field initializers and method bodies to refer fields not in order they are listed in. Unfortunately, there is no clean solution for that currently so I'm leaving a FIXME.


Repository:
  rC Clang

https://reviews.llvm.org/D44100

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===================================================================
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1019,5 +1019,28 @@
       "main.c", enumDecl(), VerificationMatcher);
 }
 
+AST_MATCHER_P(RecordDecl, hasFieldOrder, std::vector<StringRef>, Order) {
+  size_t Index = 0;
+  for (FieldDecl *Field : Node.fields()) {
+    if (Index == Order.size())
+      return false;
+    if (Field->getName() != Order[Index])
+      return false;
+    ++Index;
+  }
+  return Index == Order.size();
+}
+
+TEST(ImportDecl, ImportFieldOrder) {
+  MatchVerifier<Decl> Verifier;
+  testImport("struct declToImport {"
+             "  int b = a + 2;"
+             "  int a = 5;"
+             "};",
+             Lang_CXX11, "", Lang_CXX11, Verifier,
+             recordDecl(hasFieldOrder({"b", "a"})));
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1015,6 +1015,34 @@
   
   for (auto *From : FromDC->decls())
     Importer.Import(From);
+
+  // Reorder declarations in RecordDecls because they may have another
+  // order. Keeping field order is vitable because it determines structure
+  // layout.
+  // FIXME: This is an ugly fix. Unfortunately, I cannot come with better
+  // solution for this issue. We cannot defer expression import here because
+  // type import can depend on them.
+  const RecordDecl *FromRD = dyn_cast<RecordDecl>(FromDC);
+  if (!FromRD)
+    return;
+
+  RecordDecl *ToRD = cast<RecordDecl>(Importer.Import(cast<Decl>(FromDC)));
+
+  for (auto *FromField : FromRD->fields()) {
+    Decl *ToField = Importer.GetAlreadyImportedOrNull(FromField);
+    assert(ToRD == ToField->getDeclContext() && ToRD->containsDecl(ToField));
+    ToRD->removeDecl(ToField);
+  }
+
+  assert(ToRD->field_empty());
+
+  for (auto *FromField : FromRD->fields()) {
+    Decl *ToField = Importer.GetAlreadyImportedOrNull(FromField);
+    assert(ToRD == ToField->getDeclContext());
+    assert(ToRD == ToField->getLexicalDeclContext());
+    assert(!ToRD->containsDecl(ToField));
+    ToRD->addDeclInternal(ToField);
+  }
 }
 
 bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44100.137034.patch
Type: text/x-patch
Size: 2360 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180305/ac03665b/attachment.bin>