[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