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: > 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). 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
