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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
