ahatanak added inline comments.

Comment at: include/clang/AST/NonTrivialCStructTypeVisitor.h:30
+    if (asDerived().getContext().getAsArrayType(FT))
+      return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > Should you have this pass the array type down?  And is it really 
> > > important to do this in the generic visitor?  It seems like something you 
> > > could do in an IRGen subclass.
> > The subclasses in CGNonTrivialStruct.cpp need the size and the element type 
> > of the array to be passed to visitArray, so I think we have to pass the 
> > array type to visitArray. I guess it's possible to move this to the 
> > subclasses, but then the visit methods in the subclasses have to check 
> > whether the type is an array or not. I think we had a discussion on how 
> > arrays should be handled in this review: https://reviews.llvm.org/D41228.
> > 
> > But perhaps you have a better idea in mind?
> Well, you could "override" the visit method in the subclass, e.g.:
>   template <class Derived, class RetTy = void>
>   class CGDestructedTypeVisitor : public DestructedTypeVisitor<Derived, 
> RetTy> {
>     using super = DestructedTypeVisitor<Derived, RetTy>;
>   public:
>     using super::asDerived;
>     using super::visit;
>     template <class... Ts>
>     RetTy visit(QualType::DestructionKind DK, QualType FT, Ts &&... Args) {
>       if (asDerived().getContext().getAsArrayType(FT))
>         return asDerived().visitArray(DK, FT, std::forward<Ts>(Args)...);
>       return super::visit(DK, FT, std::forward<Ts>(Args)...);
>     }
>   };
> It's a bit more boilerplate, but I really feel like the array logic doesn't 
> belong in the generic visitor.
> About the array type: I wasn't trying to suggest that you should pass the 
> element type to visitArray, I was suggesting you could just pass the array 
> type as an `ArrayType*`, since that's what `visitArray` actually wants.
I incorporated both of your suggestions. I renamed the overloaded method 
'visit' to 'visitWithKind' so that the 'visit' method that doesn't take the 
DestructionKind doesn't get hidden by the one that takes DestructionKind and is 
defined in the derived classes. There are a couple of places in 
CGNonTrivialStruct.cpp that call the former method.

  rC Clang


cfe-commits mailing list

Reply via email to