royjacobson added inline comments.
================ Comment at: clang/lib/Sema/SemaDecl.cpp:17899 + ConstraintSatisfaction Satisfaction; + if (S.CheckFunctionConstraints(Method, Satisfaction)) + SatisfactionStatus.push_back(false); ---------------- erichkeane wrote: > royjacobson wrote: > > erichkeane wrote: > > > cor3ntin wrote: > > > > erichkeane wrote: > > > > > 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. > > > > Thanks, > > > > Follow up stupid question then, do we care about the triviality of > > > > dependant types? > > > > I think doing the check on complete non-dependant types might be a > > > > better solution than trying to do it lazily on triviality checks? > > > I would think it would be not a problem on non-dependent types. BUT > > > concepts are only allowed on templated functions (note not only on > > > function-templates!) anyway, so I don't think this would be a problem? > > Erich, I'm a bit confused by your response. I think my example demonstrates > > that for default constructors (and other SMFs) GCC and MSVC instantiate the > > constraints on class completion and **not** on demand. This is what I would > > like to do as well, if we don't have a good reason not to. (For > > destructors, performing the checks is even explicit in the standard.) > > > > Not doing this can introduce some REALLY bad edge cases. What does this do > > if we defer the triviality computation? > > > > ```c++ > > > > template <class T> > > struct Base<T> { > > Base() = default; > > Base() requires (!std::is_trivial_v<T>); > > }; > > > > struct Child : Base<Child> { }; > > ``` > > We defer the computation of the constraints on `Base`, and complete `Child` > > somehow, but if `Child` is complete then `std::is_trivial_v<Child>` should > > be well-formed, right? But we get a logical contradiction instead. > > > > > >Erich, I'm a bit confused by your response > It is entirely possible we're talking past eachother, or misunderstanding > eachothers examples. I'm totally open to that being part of this issue. > > In that example, if we calculate the triviality at '`Base` Class completion', > `Child` is not yet complete, right? So the is_trivial_v would be UB. If you > instead require `sizeof(T)`, we typically give a diagnostic. > > In this case, you'd at MINIMUM have to wait to calculate the requires-clause > until after `Child` is complete. And it isn't clear to me that we're > delaying it until then. > > That is what I intend to show with: https://godbolt.org/z/1YjsdY73P > > As far as doing it 'on demand', I definitely see your circular argument here, > but I think the is_trivial_v test is UB if we calculate it at `Base' > completion. I'm arguing for checking the constraints at the completion of `Base`, and for making `is_trivial_v/sizeof` ill formed in this context. Your GCC example is a bit misleading, I think. They check the `sizeof(T) > 0` constraint at the completion of `Base` but they just swallow the error and declare the constraint unsatisfied or something. They should've probably issued a diagnostic or something. But if you look at which constructor they choose, they choose the unconstrained one: https://godbolt.org/z/rKj4q5Yx9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128619/new/ https://reviews.llvm.org/D128619 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
