================
@@ -3761,13 +3761,17 @@ Decl
*TemplateDeclInstantiator::VisitNonTypeTemplateParmDecl(
if (IsExpandedParameterPack)
Param = NonTypeTemplateParmDecl::Create(
SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
- D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
+ D->getDepth() - (TemplateArgs.retainInnerDepths()
+ ? 0
+ : TemplateArgs.getNumSubstitutedLevels()),
D->getPosition(), D->getIdentifier(), T, TSI,
ExpandedParameterPackTypes, ExpandedParameterPackTypesAsWritten);
else
Param = NonTypeTemplateParmDecl::Create(
SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
- D->getDepth() - TemplateArgs.getNumSubstitutedLevels(),
+ D->getDepth() - (TemplateArgs.retainInnerDepths()
+ ? 0
+ : TemplateArgs.getNumSubstitutedLevels()),
----------------
erichkeane wrote:
> That's true, but the depth preservation didn't come up in that discussion,
> which would have steered us sooner to the conclusion that both instantiation
> modes have issues here.
>
> I have been thinking more about this problem.
>
> I think even depth preservation is the wrong thing here, that's not exactly
> what parameter mapping substitution asks for.
>
> What happens is that we are reparenting the lambda to the top level concept,
> which is at depth 0, so all these lambdas should end up at the same template
> depth, ie 1.
>
> So for example, a lambda nested as a constant template parameter type of
> another lambda should still have one level reduced, which is not what the
> patch is doing, which preserves all levels.
>
> Do we have test cases for that?
Hmm... this is an interesting thought. Re-writing the depth of all our AST
nodes when we re-parent might be a troublesome amount of work though, right?
Wouldn't the depth changes end up being pretty intrusive?
I THINK the 'outer retained levels' was supposed to deal with stuff like this
though, the idea was (at least as far as I could tell?) figure out the
adjustment needed for parameter mapping, though I'm not sure it was ever
effectively implemented for concepts/lambda combinations.
At the moment, I agree that the above adjustment seems a bit 'hacky', but I'm
not sure we're actually in a position to be doing this correctly without a
sizable change. I might suggest we move ahead with this and consider how we
want to handle the 'moving' of an AST node.
https://github.com/llvm/llvm-project/pull/210381
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
