martong marked 3 inline comments as done.
martong added inline comments.
================
Comment at: clang/lib/AST/ASTImporter.cpp:5109
} else { // ODR violation.
// FIXME HandleNameConflict
+ return make_error<ImportError>(ImportError::NameConflict);
----------------
a_sidorin wrote:
> Is this FIXME obsolete now?
That is not obsolete. We should call `HandleNameConflict` as we do in other
visit functions. But it is not trivial how to do that, there are open
questions which I cannot address in this PR:
* Should we pass `D->getDeclName` or rather the name of the imported
`ClassTemplateDecl` or should we import the name here (again)?
* Should we pass the template args to `HandleNameConflict`?
================
Comment at: clang/lib/AST/ASTImporter.cpp:7823
+ auto Pos = ImportedDecls.find(FromD);
+ if (Pos != ImportedDecls.end()) {
+ // Import failed after the object was created.
----------------
a_sidorin wrote:
> I see the enabled test case, but does it cover the logic in this block?
Right, that is not strictly related. I have added relevant unit test cases for
the error handling. This branch is handled in
`ErrorHappensAfterCreatingTheNodeButBeforeLinkingThatToTheAST` test.
================
Comment at: clang/lib/AST/ASTImporter.cpp:7851
+ if (!getImportDeclErrorIfAny(FromD)) {
+ // Error encountered for the first time.
+ // After takeError the error is not usable any more in ToDOrErr.
----------------
a_sidorin wrote:
> Is it possible to get this error more than once?
Yes, that can happen in cyclic imports like: ClassTemplateDecl -> TemplatedDecl
-> ClassTemplateDecl.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62373/new/
https://reviews.llvm.org/D62373
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
