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

[Alexey Sidorin via cfe-dev]

Hi Gabor,

Passing cast operation as a template argument looks overcomplicated to 
me. However, it can be used as a base for other operations, so I think 
it is worth it to include this as an overload. It can also be useful if 
an operation other than cast is required, but, unfortunately, I cannot 
remember any case where we need such behaviour.
The problem is that I cannot compile your sample. I tried to write this:

     /// Helper for importing Decls as Optionals.
     template <template <typename> class CastOp, typename DeclTy>
     Optional<DeclTy *> import(DeclTy *FromD) {
       if (auto Res = Importer.Import(FromD))
         return CastOp<DeclTy>(*Res);
       return None;
     }

     template <typename DeclTy>
     Optional<DeclTy *> importNullable(DeclTy *FromD) {
       return import<cast_or_null>(FromD);
     }

     template <typename DeclTy> Optional<DeclTy *> importNonNull(DeclTy 
*FromD) {
       return import<cast>(FromD);
     }


But there are two issues. First, cast<> has two template arguments, not 
one. Second, template instantiation cannot guess what overload of cast 
should be used in each case. I'm sorry, but after a few attempts of 
making this code to compile, I have to ask you to elaborate a bit more 
once again.

Regarding naming: what do you think about importNonNull[Into]() and 
importNullable[Into]()? So, we assume that import should return the same 
type as the argument (which, I think, is somewhat expected). Or should 
we reflect the cast it in the function name somehow?

Currently, the list of helpers look like this:

     template <typename DeclTy>
     LLVM_NODISCARD bool importNullableInto(DeclTy *&ToD, Decl *FromD);

     template <typename DeclTy>
     LLVM_NODISCARD bool importNonNullInto(DeclTy *&ToD, Decl *FromD);

     template <typename DeclTy>
     Optional<DeclTy *> importNullable(DeclTy *FromD);

     template <typename DeclTy> Optional<DeclTy *> importNonNull(DeclTy 
*FromD);

There is also another thing I am curious about: do we need to use 
dyn_casts on the return type? I'm not sure we can return a node with 
type so different from the imported node's type. Currently, there are 
some dyn_cast[_or_null]s in our code, but, possibly, they can be 
replaced with just casts, because we usually return nullptr or a pointer 
of same type as the imported one.


18.05.2018 22:47, Gábor Márton пишет:
> Of course, I think there is no need to have different functions which 
> differ only in the cast operation. We can use a template template 
> parameter for the cast operation:
>
>     template <template<typename> class CastOp, typename DeclTy>
>     LLVM_NODISCARD bool import(DeclTy *&ToD, DeclTy *FromD) {
>       if (auto Res = Importer.Import(FromD)) {
>         ToD = CastOp<DeclTy>(*Res);
>         return false;
>       }
>       return true;
>     }
>
> And then the usage:
> if (import<cast_or_null>(ToEPI.ExceptionSpec.SourceDecl,
>                  FromEPI.ExceptionSpec.SourceDecl))
>     return {};
>
> Gabor
>
> On Fri, May 18, 2018 at 8:52 PM Aleksei Sidorin <a.sidorin at samsung.com 
> <mailto:a.sidorin at samsung.com>> wrote:
> >
> > Sorry, I didn't got the idea. Could please you explain a bit more?
> >
> > 18.05.2018 13:09, Gábor Márton via cfe-dev пишет:
> > > An other idea, couldn't we just pass the cast operation as a template
> > > template parameter?
> > > _______________________________________________
> > > cfe-dev mailing list
> > > cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
> >
> > --
> > Best regards,
> > Aleksei Sidorin,
> > SRR, Samsung Electronics
> >


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180523/1724ab22/attachment.html>