[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hello Balazs,
The patch looks good in general.



================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions
----------------
balazske wrote:
> martong wrote:
> > I don't exactly see how this test is related.
> I do not remember exactly why this test was added but probably the problem is structural equivalence related: The flags are not imported correctly for the first time, and at the second import structural match fails and a new Decl is created instead of returning the existing one. This test fails if the change is not applied.
Should we consider isExplicitlyDefaulted() when computing structural equivalence?


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5242
+  // are merged into one chain.
+  auto GetDeclToImport = [this](const char *File) {
+    Decl *FromTU = getTuDecl(
----------------
`const char *` -> `StringRef`?


================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5246
+        struct X { };
+        void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+        )",
----------------
Can we remove the function body or reduce it to 'X x'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65999