martong added a comment.
Nice catch!
================
Comment at: clang/lib/AST/ASTImporter.cpp:8789-8791
+ // The import path contains child nodes first.
+ // (Parent-child relationship is used here in sense of import
+ // dependency.)
----------------
I think, this comment would be better to describe the declaration of
`PrevFromDi`.
================
Comment at: clang/lib/AST/ASTImporter.cpp:8792-8794
+ if (!isa<TagDecl>(FromDi))
+ if (auto *FromDiDC = dyn_cast<DeclContext>(FromDi))
+ if (FromDiDC->containsDecl(PrevFromDi))
----------------
We should elevate this logic and the one in `ImportDeclContext` that uses
`consumeError` into a common abstraction. I.e. making them closer to each other
in a class.
Something like (draft, did not test it, might not compile):
```
class ErrorHandlingStrategy {
static bool accumulateChildErrors(DeclContext*);
static bool accumulateChildErrors(Decl *FromDi, Decl* Parent); // Use this
one when traversing through the import path!
public:
static Error handleDeclContextError(Error Err, DeclContext* FromDC) {
Error ChildErrors = Error::success();
if (Err && accumulateChildErrors(FromDC))
ChildErrors = joinErrors(std::move(ChildErrors), std::move(Err));
else
consumeError(std::move(Err));
return ChildErrors
}
};
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122525/new/
https://reviews.llvm.org/D122525
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
