[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

Aleksei Sidorin via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 24 12:23:02 PDT 2019


a_sidorin added a comment.

Hi Balazs,

The looks mostly good to me.



================
Comment at: lib/AST/ASTImporter.cpp:3440
 
-  for (const auto *Attr : D->attrs())
-    ToIndirectField->addAttr(Importer.Import(Attr));
----------------
There is the same deletion in D53757.


================
Comment at: lib/AST/ASTImporter.cpp:8550
+    if (ExpectedType ToFromOrErr = Import_New(From)) {
+      if (ToContext.hasSameType(*ToFromOrErr, To))
+        return true;
----------------
Wow, we import types instead of just checking them for structural equivalence. That's OK to leave it in the patch as-is but looks pretty strange. Maybe this even deserves a FIXME.


================
Comment at: unittests/AST/ASTImporterTest.cpp:146
+             << "Import failed, error: \"" << Err << "\"!";
+      llvm::consumeError(std::move(Err));
+    }
----------------
Dead code?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55049/new/

https://reviews.llvm.org/D55049





More information about the cfe-commits mailing list