[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21eb1af469c3: [Concepts] Implement overload resolution for destructors (P0848) (authored by royjacobson). Repository: rG LLVM Github Monorepo

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson updated this revision to Diff 437940. royjacobson added a comment. Fix the AST test on windows and rebase on HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126194/new/ https://reviews.llvm.org/D126194 Files:

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1430 + /// out of which one will be selected at the end. + /// This is called separately from addedMember because it has to be deferred + /// to the completion of the class.

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/AST/DeclCXX.h:1430 + /// out of which one will be selected at the end. + /// This is called separately from addedMember because

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson added inline comments. Comment at: clang/docs/ReleaseNotes.rst:434 +- As per "Conditionally Trivial Special Member Functions" (P0848), it is + now possible to overload destructors using concepts. Note that the rest erichkeane wrote: > Do we have

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

erichkeane added a comment. I probably need to spend more time on this at one point, but first look seemed fine to me. I think the approach is about right, and the solution is there. @aaron.ballman is messing with the constexpr-ness of SM functions, so there is likely some collaboration that

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson updated this revision to Diff 437656. royjacobson added a comment. Update the PR to now handle triviality, which turned out to require a different approach. On the bright side it is now easier to see how to implemenet the rest of P0848. Added an AST dump test that checks the

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

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(Decl); +if (DD && DD->isEligibleOrSelected()) erichkeane wrote: > What cases happen

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

erichkeane added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:1901 + for (auto* Decl : R) { +auto* DD = dyn_cast(Decl); +if (DD && DD->isEligibleOrSelected()) What cases happen when the lookup for a destructor name ends up not being a

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson updated this revision to Diff 431589. royjacobson added a comment. Update the approach to use an AST property, and enable this for all language variants (not just CPP20). There's still one test failing because OpenCL have got their own destructor thing going, I'll try to see what I

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

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

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

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

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

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

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson added a comment. In D126194#3531280 , @cor3ntin wrote: > In D126194#3531267 , @erichkeane > wrote: > >> How much of P0848 is missing after this? If nothing/not much, should we >> update

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

cor3ntin added a comment. In D126194#3531267 , @erichkeane wrote: > How much of P0848 is missing after this? If nothing/not much, should we > update cxx_status.html as well? P0848 applies to all special member function. At best we could mark it

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

erichkeane added a comment. How much of P0848 is missing after this? If nothing/not much, should we update cxx_status.html as well? Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2330 + /// To achieve that, we remove all non-selected destructors from the AST, + ///

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson updated this revision to Diff 431333. royjacobson added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126194/new/ https://reviews.llvm.org/D126194 Files: clang/docs/ReleaseNotes.rst

[PATCH] D126194: [Concepts] Implement overload resolution for destructors (P0848)

royjacobson created this revision. Herald added a project: All. royjacobson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch implements a necessary part of P0848, the overload resolution for destructors. It is now possible to