[PATCH] D76038: PR45000: Use Sema::SetParamDefaultArgument in TransformLambdaExpr

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/TreeTransform.h:12162
 
   for (unsigned I = 0, NumParams = NewCallOperator->getNumParams();
        I != NumParams; ++I) {
----------------
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.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76038/new/

https://reviews.llvm.org/D76038