[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

This looks good to me.

I *think* landing things without doing silent sfinae for incomplete types is okay-ish, but I'd like to here from other folks.

@royjacobson Have you looked into that? Do you know how involved would it be?

Either way, we should make sure that if that bit is dealt with separately, we have an issue or something to track it, maybe a note in `cxx_status`.



================
Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 <https://wg21.link/p0848r3>`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1496>`_
+  and `DR1734 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1734>`_.
----------------
royjacobson wrote:
> h-vetinari wrote:
> > Is that lack of compliance worth a note in `cxx_status`?
> I'm not very opinionated about this, but I tend to not significant enough. I mean, 7 years later and only MSVC have even bothered to implement them.
We might has well, I think it's a good way to not lose track of it.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+    return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+                           M2->getParamDecl(0)->getType()))
----------------
royjacobson wrote:
> royjacobson wrote:
> > shafik wrote:
> > > What happens if we have further parameters with default arguments? Unless I am missing something they are still special member functions but the proposal does not seem to cover them.
> > That's an excellent question.
> > 
> > I'm not sure what to do about default arguments. In a context where the additional parameters matter, you're not using them as constructors anymore, right? So why would this affect the type traits?
> > On the one hand [over.match.best] is against this idea of comparing constraints when the parameters differ. So also every context where this actually matters the overload resolution would probably be ambiguous anyway?
> > 
> > @BRevzin, what do you think? Is the wording intentional to include copy/move constructors with default arguments as well?
> > 
> > I checked with GCC and they seem to handle default arguments separately: https://godbolt.org/z/1ch3M7MjP
> > 
> FWIW, I filed https://github.com/cplusplus/CWG/issues/110
> 
> I'm not on the reflector myself, so I don't know if there's been a follow-up there.
Yes, i think it's reasonable to not concerns ourselves with default parameters until wg21 decides otherwise


================
Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:200
+// FIXME: We should not throw an error, instead SFINAE should make the constraint
+// silently unsatisfied. See [temp.constr.constr]p5
+template <class T>
----------------
Have you been able to investigate that?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128619/new/

https://reviews.llvm.org/D128619