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.
----------------
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.
================
Comment at: clang/lib/AST/ASTImporter.cpp:2088
+ // Set to false here to force "completion" of the record.
+ To->setCompleteDefinition(false);
if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
----------------
My fix was not correct. This line was added to make a test
`CompleteRecordBeforeImporting` not fail but it makes the original fix not
work. Something other is needed.
================
Comment at: clang/unittests/AST/ASTImporterTest.cpp:7485
+ static void foo(A x) {
+ (void)&"text"[x.idx];
+ }
----------------
shafik wrote:
> The member function body should be considered `complete-class context` so the
> correct thing to do would be have all the fields laid out by this point.
My initial plan was to import first all fields, then everything else. But it is
possible that a field has reference to the same record before the import of all
fields finishes (like in the second test) so this can not work in all cases
(and it caused other test failures).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116155/new/
https://reviews.llvm.org/D116155
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
