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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I believe the calculation of the constraints here means this isn't conforming with concepts.  I believe this means we have to delay calculating the constraint differences until this property is actually queried, and cannot do it early like this has.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:17874
+      return true;
+    if (M1->getParamDecl(0)->getType() != M2->getParamDecl(0)->getType())
+      return false;
----------------
I don't think you'd want to compare with equality here, it ends up not desugaring/etc.  I believe `ASTContext::HasSameType` should be what you want.  


================
Comment at: clang/lib/Sema/SemaDecl.cpp:17876
+      return false;
+    if (M1->getThisType() != M2->getThisType())
+      return false;
----------------
Probably same here.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+      ConstraintSatisfaction Satisfaction;
+      if (S.CheckFunctionConstraints(Method, Satisfaction))
+        SatisfactionStatus.push_back(false);
----------------
This seems problematic, doesn't it?  Checking this constraint will (once I figure out how to get deferred instantiation to work) cause instantiation, which can cause issues with incomplete types/CRTP/etc.

I think the result is that we cannot 'calculate' this until it is queried, else we will cause incorrect errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619