[PATCH] D47632: [ASTImporter] Refactor Decl creation
Gabor Marton via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 26 09:31:14 PDT 2018
martong added inline comments.
================
Comment at: lib/AST/ASTImporter.cpp:1659
+ AccessSpecDecl *ToD;
+ std::tie(ToD, AlreadyImported) = CreateDecl<AccessSpecDecl>(
+ D, Importer.getToContext(), D->getAccess(), DC, Loc, ColonLoc);
----------------
a.sidorin wrote:
> As I see, all usage samples require having a variable AlreadyImported used only once. How about changing the signature a bit:
> ```bool getOrCreateDecl(ToDeclTy *&ToD, FromDeclT *FromD, Args &&... args)```
> with optional `LLVM_NODISCARD`? (Naming is a subject for discussion).
> This signature also allows us to omit template arguments because we pass an argument of a templated type. So, the call will look like this:
> ```AccessSpecDecl *ToD;
> if (getOrCreateDecl(ToD, D, Importer.getToContext(), D->getAccess(), DC, Loc, ColonLoc))
> return ToD;```
I like your idea, because indeed we could spare the repetition of the type in most cases. However, in one particular case we have to use the original version: in `VisitTypedefNameDecl` where we assign either a `TypeAliasDecl *` or a `TypedefDecl *` to a pointer to their common base class, `TypedefNameDecl`.
So, I think it is possible to add an overload with the signature `(ToDeclTy *&ToD, FromDeclT *FromD, Args &&... args)` and we could use that in the simple cases (which is the majority).
About the naming I was thinking about `getAlreadyImportedOrCreateNewDecl`, but this appears to be very long. So if you think this is way too long then I am OK with the shorter `getOrCreateDecl`.
Repository:
rC Clang
https://reviews.llvm.org/D47632
More information about the cfe-commits
mailing list