[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.

Balázs Kéri via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 5 02:25:56 PST 2022


balazske added inline comments.


================
Comment at: clang/lib/AST/ASTImporter.cpp:2021
+  // fields imported) at that time without multiple AST import passes.
+  To->setCompleteDefinition(true);
   // Complete the definition even if error is returned.
----------------
balazske wrote:
> shafik wrote:
> > So `DefinitionCompleterScopeExit` will run `To->setCompleteDefinition(false);`  after this function exits but this will be in effect during the import the base classes. I don't see how the tests you added hit that code.
> Should the test `CompleteRecordBeforeImporting` not do the  import in minimal mode? The comment says minimal but in `ASTImporter` minimal mode is not set. The test will fail because this added line. But I think it should work to call the `To->setCompleteDefinition` here only in non-minimal mode.
And remove the later added line 2088 `To->setCompleteDefinition(false);`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116155



More information about the cfe-commits mailing list