martong added a reviewer: balazske.
martong marked 6 inline comments as done.
martong added inline comments.
================
Comment at: clang/lib/AST/ASTImporter.cpp:7830
if (ToD) {
+ // Already imported (possibly from another TU) and with an error.
+ if (auto Error = SharedState->getImportDeclErrorIfAny(ToD))
----------------
a_sidorin wrote:
> I'm not sure if it is safe to compare declarations from different TUs by
> pointers because they belong to different allocators.
In the `SharedState` we keep pointers only from the "to" context.
================
Comment at: clang/lib/AST/ASTImporter.cpp:7831
+ // Already imported (possibly from another TU) and with an error.
+ if (auto Error = SharedState->getImportDeclErrorIfAny(ToD))
+ return make_error<ImportError>(*Error);
----------------
a_sidorin wrote:
> getImportDeclErrorIfAny() is usually called for FromD, not for ToD. Is it
> intentional?
`ASTImporter::getImportDeclErrorIfAny()` is different from
`ASTImporterSharedState::getImportDeclErrorIfAny()`.
The former keeps track of the erroneous decls from the "from" context.
In the latter we keep track of those decls (and their error) which are in the
"to" context.
The "to" context is common for all ASTImporter objects in the cross translation
unit context.
They share the same ASTImporterLookupTable object too. Thus the name
ASTImporterSharedState.
================
Comment at: clang/lib/AST/ASTImporter.cpp:7924
+ if (auto Error = SharedState->getImportDeclErrorIfAny(ToD))
+ return make_error<ImportError>(*Error);
+
----------------
a_sidorin wrote:
> I don' think that an import failure from another TU means that the import
> from the current TU will fail.
It should. At least if we map the newly imported decl to an existing but
already messed up decl in the "to" context. Please refer to the added test case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62376/new/
https://reviews.llvm.org/D62376
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
