[cfe-dev] [RFC] Improving import failure checking strategy inside ASTImporter

[George Karpenkov via cfe-dev]

Hi Aleksei,
>>>> 
>>>> The problem of improving readability of these checks is a bit harder. The
>>>> most straightforward approach is to create a macro like:
>>>> 
>>>> #define IMPORT_PTR_INTO_VAR(VarName, Type, From) \
>>>>   Type *VarName = nullptr;                                 \
>>>>   {                                                        \
>>>>     auto MacroImportRes = Importer.Import(From);           \
>>>>     if (!MacroImportRes)                                   \
>>>>       return MacroImportRes.getError();                    \
>>>>     VarName = *MacroImportRes;                             \
>>>>   }
>>>> 
>>>> Same for non-pointer type. So, the final code will look like this:
>>>> 
>>>> ErrorOr<Expr *> ASTNodeImporter::VisitCXXMemberCallExpr(CXXMemberCallExpr
>>>> *E) {
>>>>   IMPORT_QUAL_TYPE_INTO_VAR(T, E->getType())
>>>>   IMPORT_PTR_INTO_VAR(ToFn, Expr, E->getCallee())
>>>> 
>>>>   SmallVector<Expr *, 4> ToArgs(E->getNumArgs());
>>>>   IMPORT_CONTAINER(E->arguments(), ToArgs);
>>>> 
>>>>   return new (Importer.getToContext()) CXXMemberCallExpr(
>>>>         Importer.getToContext(), ToFn, ToArgs, T, E->getValueKind(),
>>>>         Importer.Import(E->getRParenLoc()));
>>>> }
>>>> 
>>>> But macros don't look clean. Unfortunately, I still haven't found a better
>>>> option so any suggestions are very welcome.

Macros are indeed ugly, and exceptions are even worse.
I think a better solution would be to return an Optional, and make `Importer.Import` accept an out-parameter `Error`, passed by reference.
Then you could write:

Optional<Expr*> VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
  Error e;
  if (auto T = Importer.import(E->getType(), e))
    if (auto ToFn = Importer.import(E->getCallee(), e))
      if (auto ToArgs = Importer.import(E->arguments(), e)
        return new (Importer.getToContext()) CXXMemberCallExpr(…)
  outE = e;
  return none;
}

That’s even more concise than your example, explicit and does not need macros.

This would be less verbose in C++17 where you would not need nested if’s,
but sadly we don’t have that in LLVM yet.	

George
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180501/e9da97e5/attachment.html>