erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaConcept.cpp:773
+ // ConstrExpr for the inner template will properly adjust the depths.
+ if (isa<CXXRecordDecl>(ND) && isa<CXXRecordDecl>(OtherND))
+ ForConstraintInstantiation = true;
----------------
alexander-shaposhnikov wrote:
> erichkeane wrote:
> > alexander-shaposhnikov wrote:
> > > erichkeane wrote:
> > > > Hmm... this seems really strange to have to do.
> > > > `ForConstraintInstantiation` shouldn't be used here, the point of that
> > > > is to make sure we 'keep looking upward' once we hit a spot we normally
> > > > stop with. What exactly is the issue that you end up running into
> > > > here? Perhaps I can spend some time debugging what we should really be
> > > > doign.
> > > yeah, I agree. I haven't found a proper solution or at least a better
> > > workaround (but would be happy to).
> > > This kicks in for the case
> > >
> > > ```
> > > template <class T0>
> > > concept Constraint = true;
> > >
> > >
> > > template<Constraint T1>
> > > struct Iterator {
> > > template <Constraint T2>
> > > friend class Iterator;
> > > void operator*();
> > > };
> > >
> > > Iterator<char*> I2;
> > > ```
> > > yeah, I agree. I haven't found a proper solution or at least a better
> > > workaround (but would be happy to).
> > > This kicks in for the case
> > >
> > > ```
> > > template <class T0>
> > > concept Constraint = true;
> > >
> > >
> > > template<Constraint T1>
> > > struct Iterator {
> > > template <Constraint T2>
> > > friend class Iterator;
> > > void operator*();
> > > };
> > >
> > > Iterator<char*> I2;
> > > ```
> >
> > Alright, well, I should have time later in the week to poke at this,
> > perhaps I can come up with something better? I DO remember self-friend is
> > a little wacky, and I spent a bunch of time on it last time.
> Ok, sounds good + maybe Richard will give it another look.
So IMO, `ForConstraintInstantiation` should be 'true' always, and that makes
those examples pass. However, I'm now seeing that it causes a failure in the
concepts-out-of-line-def.cpp file.
I took the example of `foo3`:
```
template<typename T0> concept C = true;
template <typename T1>
struct S {
template <typename F3> requires C<F3>
void foo3(F3 f); // #1
};
template <typename T4>
template <typename F6> requires C<F6>
void S<T4>::foo3(F6 f) {} // #3
```
Which, seems to require `ForConstraintInstantiation` to be false to pass.
However, I don't think this is correct. This is only working because when
evaluating the in-line one (#1 above!) its skipping the application of `T1`,
which is wrong.
However, I think the problem here is that the `out of line` version (#3) is not
applying the T4 like it should be. SO, I think the `HandleFunctionTemplateDecl`
I provided you earlier needs modification.
FIRST, though not related to this, I think we might need to add
`FunctionTemplateDecl::getInjectedTemplateArgs` to the `Result`, but only
because that 'sounds right' to me? IDK what other problem that would cause,
but it is worth evaluating/saving for later. It might just not matter, since
we're treating them as equal at the moment, I don't think injecting them would
cause anything.
Secondly: I see that the we can get to the `T4` via the
`FTD->getTemplatedDecl()->getQualifier()->getAsType()->getAs<TemplateSpecializationType>()->template_arguments()`.
HOWEVER, the problem that comes with picking THOSE up, is that it ALSO applies
with a `FunctionTemplateDecl` inside of an out-of-line
`ClassTemplateSpecializationDecl` (which doesn't NEED the specialization's
template args).
SO I think the fix here is something in `HandleFunctionTemplateDecl` to make
sure we pick up the right list of template arguments. I haven't figured out
the magic incantation to make it work unfortunately, perhaps @rsmith has some
hints? Else I'll keep poking at it.
BUT, my debugging has shown me that I'm quite convinced the setting
`ForConstraintInstantiation ` to false is incorrect.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146178/new/
https://reviews.llvm.org/D146178
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
