[PATCH] D65999: [ASTImporter] Import additional flags for functions.
    Balázs Kéri via Phabricator via cfe-commits 
    cfe-commits at lists.llvm.org
       
    Mon Aug 12 00:51:27 PDT 2019
    
    
  
balazske marked 2 inline comments as done.
balazske added inline comments.
================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions
----------------
a_sidorin wrote:
> 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?
We may use `isExplicitlyDefaulted` and `isDeletedAsWritten` and `isVirtualAsWritten` but in another patch.
================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:5246
+        struct X { };
+        void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+        )",
----------------
a_sidorin wrote:
> Can we remove the function body or reduce it to 'X x'?
The `delete x3` is needed to get a destructor for `X` generated. But the test is about testing the implicit functions and I think it is better to have more of them. This code is there to have these functions generated so I do not like remove of it.
Repository:
  rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65999/new/
https://reviews.llvm.org/D65999
    
    
More information about the cfe-commits
mailing list