ChuanqiXu added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:489
+ // Attn Reviewers: we need to do this for the function constraints for
+ // comparison of constraints to work, but do we also need to do it for
+ // CheckInstantiatedFunctionConstraints? That one is more difficult, but we
----------------
erichkeane wrote:
> ChuanqiXu wrote:
> > erichkeane wrote:
> > > ChuanqiXu wrote:
> > > > Would you mind to elaborate for the issue "function constraints for
> > > > comparison of constraints to work" more? Maybe it is said in previous
> > > > messages, but the history is hard to follow...
> > > Yep, this one is difficult :/ Basically, when we instantiate the
> > > constraints at 'checking' time, if the function is NOT a template, we
> > > call `CheckFunctionConstraints`. When we go to check the 'subsumption'
> > > type things with a fully instantiated version, they fail if they are
> > > dependent (as it is expected that way). See the
> > > `setTrailingRequiresClause` here.
> > >
> > > HOWEVER, it seems we ALWAYS pick constraints off the primary template,
> > > rather than the 'stored' version of the constraint on the current
> > > declaration. I tried to do something similar here for those cases, but
> > > 1: it had no effect seemingly, and 2: it ended up getting complicated in
> > > many cases (as figuring out the relationship between the constraints in
> > > the two nodes was difficult/near impossible).
> > >
> > > I opted to not do it, and I don't THINK it needs to happen over there,
> > > but I wanted to point out that I was skipping it in case reviewers had a
> > > better idea.
> > >
> > >
> > Let me ask some questions to get in consensus for the problem:
> >
> > > Basically, when we instantiate the constraints at 'checking' time, if the
> > > function is NOT a template, we call CheckFunctionConstraints.
> >
> > I think the function who contains a trailing require clause should be
> > template one. Do you want to say independent ? Or do you refer the
> > following case?
> >
> > ```C++
> > template<typename T> struct A {
> > static void f(int) requires xxxx;
> > };
> > ```
> >
> > And the related question would be what's the cases that
> > `CheckFunctionConstraints` would be called and what's the cases that
> > `CheckinstantiatedFunctionTemplateConstraints` would be called?
> >
> > > When we go to check the 'subsumption' type things with a fully
> > > instantiated version, they fail if they are dependent (as it is expected
> > > that way).
> >
> > If it is fully instantiated, how could it be dependent?
> >
> > > HOWEVER, it seems we ALWAYS pick constraints off the primary template,
> > > rather than the 'stored' version of the constraint on the current
> > > declaration.
> >
> > 1. What is `we`? I mean what's the function would pick up the constraints
> > from the primary template.
> > 2. Does `the stored version of the constraint` means the trailing require
> > clause of FD? Would it be the original one or `Converted[0]`.
> >>Or do you refer the following case?
>
> Yeah, thats exactly the case I am talking about. A case where the function
> itself isn't a template, but is dependent.
>
> >>And the related question would be what's the cases that
> >>CheckFunctionConstraints would be called and what's the cases that
> >>CheckinstantiatedFunctionTemplateConstraints would be called?
>
> The former when either the function is not dependent at all, OR it is
> dependent-but-fully-instantiated (like in the example you gave). All other
> cases (where the template is NOT fully instantiated) calls the other function.
>
> >>If it is fully instantiated, how could it be dependent?
>
> By "Fully Instantiated" I mean "everything except the constraints (and
> technically, some noexcept)", since we are deferring constraint checking
> until that point. A bunch of the changes in this commit STOP us from
> instantiating the constraint except when checking, since that is the point of
> the patch! SO, the constriant is still 'dependent' (like in your example
> above).
>
>
> >>What is we? I mean what's the function would pick up the constraints from
> >>the primary template.
>
> Royal 'we', or clang. The function is `getAssociatedConstraints`.
>
> >>Does the stored version of the constraint means the trailing require clause
> >>of FD? Would it be the original one or Converted[0].
>
> The 'stored' version (that is, not on the primary template) is the one that
> we partially instantiated when going through earlier instantiations. In the
> case that it was a template, we seem to always ignore these instantiated
> versions. IN the case where it is NOT a template, we use that for checking
> (which is why Converted[0] needs replacing here).
>
I see roughly. The implementation looks a little bit odd to me.. But I don't
find better solutions..
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:60-61
const NamedDecl *D, const TemplateArgumentList *Innermost,
- bool RelativeToPrimary, const FunctionDecl *Pattern) {
+ bool RelativeToPrimary, const FunctionDecl *Pattern, bool LookBeyondLambda,
+ bool IncludeContainingStructArgs) {
// Accumulate the set of template argument lists in this structure.
----------------
Would you elaborate more for `LookBeyondLambda` and
`IncludeContainingStructArgs`? It confuses me since I couldn't find `Lambda` or
`Struct` from the context of use point.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119544/new/
https://reviews.llvm.org/D119544
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
