royjacobson planned changes to this revision. royjacobson added inline comments.
================ Comment at: clang/lib/AST/DeclCXX.cpp:1901 + for (auto* Decl : R) { + auto* DD = dyn_cast<CXXDestructorDecl>(Decl); + if (DD && DD->isEligibleOrSelected()) ---------------- erichkeane wrote: > What cases happen when the lookup for a destructor name ends up not being a > destructor on a RecordDecl type? I guess I could see this happening with a > pseudo-destructor, but that isn't a class type. > > It's a bit silly, but it's because we can have template destructors that we added to the AST even though they're invalid (it ends up as a FunctionTemplateDecl). I noticed it because of SemaTemplate/destructor-template.cpp ================ Comment at: clang/lib/Sema/SemaDecl.cpp:17841 + if (CXXRecord && !CXXRecord->isDependentType()) + ComputeSelectedDestructor(*this, CXXRecord); + ---------------- erichkeane wrote: > How does all of this play with the 'defaulted destructor is constexpr' stuff? > We end up storing facts like that, destructor > triviality/irrelevance/defaulted-and-constexpr/deleted/etc as a bit in the > Decl, rather than calculating it. Can this feature (Allowing overloading) > cause those to be inaccurate? > > That is, could we do something like use a requires clause to select between a > trivial, irrelevant, and constexpr destructor? Do we need to make sure we > update those too? I would expect that the destructor here would need to > store its OWN trivaility/relevance/constexpr/etc here, so we can then update > those flags on the type once it is selected. > You're right, I also found `canPassInRegisters` that will need to be adjusted a bit so it seems I need to write some codegen tests to make sure this gets the ABI correctly. I won't have much time in the coming month, I hope to get to it by the start of July (but if someone wants to pick this up in the meantime feel free to do so). 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