[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/include/clang/AST/Decl.h:1944-1948
+  /// For non-templates this value will be NULL, unless this non-template
+  /// function declaration was declared directly inside of a function template,
+  /// in which case this will have a pointer to a FunctionDecl, stored in the
+  /// NamedDecl. For function declarations that describe a function template,
+  /// this will be a pointer to a FunctionTemplateDecl, stored in the NamedDecl.
----------------
ChuanqiXu wrote:
> erichkeane wrote:
> > ChuanqiXu wrote:
> > > Could we give a new abstract like `XXXInfo` to replace `NamedDecl*` here? The NamedDecl* covers a lot of things. It looks more consistent.
> > All of those 'Info' types contain significantly more information, which we really don't have a need for here.  Basically all this patch is doing is using the FunctionTemplateDecl* that was already in the list and generalizing it a bit.  AND unfortunately, the only commonality between FunctionTemplateDecl and FunctionDecl is NamedDecl.  
> > 
> > So any type we would use would end up causing an additional allocation here, and make its use require us to unpack it :/
> > 
> > I guess what I'm saying, is I'm not sure what that would look like in a way that wouldn't make this worse.  Do you have an idea you could share that would improve that?  
> > 
> > 
> My idea would be something like:
> 
> ```C++
> llvm::PointerUnion<FunctionDecl *, FunctionTemplateDecl*, MemberSpecializationInfo *,
>                      FunctionTemplateSpecializationInfo *,
> ```
> 
> > So any type we would use would end up causing an additional allocation here, and make its use require us to unpack it :/
> 
> This is a union so it wouldn't cause additional allocations?
> 
I tried doing FunctionDecl and FunctionTemplateDecl at one point, but we endd up running out of bits for the pointer-union to work on 32 bit builds.  Otherwise I would have done that.

I misunderstood the 'Info' thing and thought you wanted a new wrapper type (an 'Info' type), which would need to be allocated and contain just a pointer, which is why I was thinking about needing allocations.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126907/new/

https://reviews.llvm.org/D126907