aaronpuchert marked 2 inline comments as done.
aaronpuchert added inline comments.
================
Comment at: clang/lib/Sema/TreeTransform.h:12162
for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
I != NumParams; ++I) {
----------------
rsmith wrote:
> rsmith wrote:
> > This logic shouldn't even be here in the first place.. this should all be
> > handled by `SubstParmVarDecl` in `SemaTemplateInstantiate`. It's wrong for
> > `TreeTransform` to be assuming that `TransformFunctionProtoType` will have
> > built new function parameters -- it doesn't do so by default, so in the
> > non-template-instantiation case, this is clobbering the default argument
> > information on the original lambda. And in the template instantiation case,
> > this is clobbering the default argument information (including
> > initialization) that `SubstParmVarDecl` already did.
> >
> > Please try deleting this loop entirely. If there are still problems (and I
> > think there are: I don't think we handle delayed instantiation for default
> > argument in generic lambdas properly), we should address them by changing
> > how we do default argument instantiation in `SubstParmVarDecl`. In
> > particular, where that function calls `SetParamDefaultArgument` after
> > calling `SubstExpr`, we should presumably instead be calling
> > `setUninstantiatedDefaultArg` on the parameter if it's a parameter of a
> > generic lambda (we still need to instantiate the default argument with the
> > template arguments of the actual lambda call operator).
> Hmm, what I wrote above isn't entirely true -- `TreeTransform` does create
> new parameters by default. But nonetheless, it's the responsibility of
> `TransformFunctionTypeParam` to set up the default argument, not the
> responsibility of this code, so we shouldn't be partially overwriting its
> results here. (Perhaps we should move the substitution into default arguments
> from `SubstParmVarDecl` into `TreeTransform::TransformFunctionTypeParam`,
> though I think that can be done separately from this bugfix.)
Thanks for your hints! I was already somewhat suspicious of that loop. Removing
it works indeed pretty well, but I'm running into an issue with variable
templates. Default arguments are usually only instantiated when needed by a
call, which happens in `Sema::CheckCXXDefaultArgExpr`. Now in the following
code, the `MultiLevelTemplateArgumentList` returned from
`Sema::getTemplateInstantiationArgs` is empty instead of being `[int]`:
```
template <typename T>
int k = [](T x = 0.0) -> int { return x; }();
template int k<int>;
```
Looking at `getTemplateInstantiationArgs`, that's not very surprising: if we
are a `DeclContext`, we go through the contexts and collect all template
arguments from those, but a variable template is not a context. The variable
template case is handled right at the beginning, but only if the declaration
itself is a variable template.
Meaning that deep inside the initializer of a variable template, we don't get
its template parameters.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76038/new/
https://reviews.llvm.org/D76038
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
