royjacobson 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 ---------------- 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) 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
