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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+      ConstraintSatisfaction Satisfaction;
+      if (S.CheckFunctionConstraints(Method, Satisfaction))
+        SatisfactionStatus.push_back(false);
----------------
cor3ntin wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > 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.
> > Making this queried on demand is a relatively big change to how we handle type triviality, so I want to be sure we actually need to do this to be conformant.
> > 
> > When I started working on this I checked what GCC does and it instantiates those constraints during class completion as well. For example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> > 
> > So maybe it's OK to check the constraints of SMFs specifically?
> > 
> I think this is done on completeness already in this patch, unless i misunderstood the code.
> I don't think doing it on demand is a great direction, as this does not only affect type traits but also code gen, etc. It would create instanciations in unexpected places. wouldn't it.
> Does the standard has wording suggesting it should be done later than on type completeness?
The problem, at least for the deferred concepts, is that it breaks in the CRTP as required to implement ranges.  So something like this: https://godbolt.org/z/hPqrcqhx5 breaks.

I'm currently working to 'fix' that, so if this patch causes us to 'check' constraints early, it'll go back to breaking that example.  The example that Roy showed seems to show that it is actually checking 'delayed' right now (that is, on demand) in GCC/MSVC.  I don't know of the consequence/standardeeze that causes that though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619