erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2330 + /// To achieve that, we remove all non-selected destructors from the AST, + /// which is a bit unusual. We can't let those declarations be in AST and rely + /// on LookupSpecialMember to return the correct declaration because a lot of ---------------- royjacobson wrote: > erichkeane wrote: > > royjacobson wrote: > > > cor3ntin wrote: > > > > erichkeane wrote: > > > > > I don't think this ends up being acceptable (removing them from the > > > > > AST). Instead, we should probably mark them as "invalid" and update > > > > > getDestructor to only return the 'only' destructor, or the first > > > > > not-invalid one. > > > > I'd rather we stay consistent with the wording, keep track of a > > > > selected destructor which would be returned by `getDestructor`. it's > > > > more surgery but it's a lot cleaner imo > > > Corentin, do you suggest doing this in CXXRecordDecl explicitly? > > > > > > Erich - I agree, this seems like a better solution, although this is a > > > bit of abuse to 'invalid' in the case of less-constrained candidates. > > > Ideally we might want another AST property of 'eligible' and keep track > > > of things there, but I don't really know the AST code well enough to do > > > it. > > I like the idea of marking them eligible that way. If this JUST has to do > > with Destructors for now, adding a bit to `CXXDestructorDecl` is a pretty > > trivial thing to do. I'm pretty sure it is defined in `DeclCXX.h`. At that > > point, we just need to make sure it is checked during 'getDestructor' (or > > at least, around any calls to that). > I think we'll need it soon for the other SMF as well. (Or maybe even for all > member functions if CWG2421 is ever resolved). > > So maybe I should just take some time to look at `FunctionDeclBitfields` and > see if I can update it. (I hope updating the ast printers isn't too hard) I'm pretty sure there is room in there. Updating the printers should be pretty easy as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126194/new/ https://reviews.llvm.org/D126194 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
