[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

[Tom Honermann via Phabricator via cfe-commits]

tahonermann requested changes to this revision.
tahonermann added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:19085-19092
+  // If the variable is used in a default argument expression of a lambda call
+  // operator, switch to the enclosing context to sync up with the function
+  // scope.
+  if (isLambdaCallOperator(DC))
+    if (auto *PVD = dyn_cast_or_null<ParmVarDecl>(
+            ExprEvalContexts.back().ManglingContextDecl))
+      if (PVD->getDeclContext() == DC)
----------------
I'm struggling to understand the change. If I'm following correctly, it looks like we're trying to capture the variable used in the default argument. If so, that seems wrong; I think we shouldn't even be trying to capture a variable for such usage.

I jumped into a debugger to poke around a bit. Perhaps the right change is to detect the use in a default argument in the code below so that the call to `getCapturedDeclRefType()` can be skipped for a non-ODR use.
  /localdisk2/thonerma/llvm-project/clang/lib/Sema/SemaExpr.cpp:
   3265 ExprResult Sema::BuildDeclarationNameExpr(
   3266     const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo, NamedDecl *D,
   3267     NamedDecl *FoundD, const TemplateArgumentListInfo *TemplateArgs,
   3268     bool AcceptInvalidDecl) {
  .....
   3326   switch (D->getKind()) {
  .....
   3389   case Decl::Var:
   3390   case Decl::VarTemplateSpecialization:
   3391   case Decl::VarTemplatePartialSpecialization:
   3392   case Decl::Decomposition:
   3393   case Decl::OMPCapturedExpr:
   3394     // In C, "extern void blah;" is valid and is an r-value.
   3395     if (!getLangOpts().CPlusPlus && !type.hasQualifiers() &&
   3396         type->isVoidType()) {
   3397       valueKind = VK_PRValue;
   3398       break;
   3399     }
   3400     [[fallthrough]];
   3401
   3402   case Decl::ImplicitParam:
   3403   case Decl::ParmVar: {
   3404     // These are always l-values.
   3405     valueKind = VK_LValue;
   3406     type = type.getNonReferenceType();
   3407
   3408     // FIXME: Does the addition of const really only apply in
   3409     // potentially-evaluated contexts? Since the variable isn't actually
   3410     // captured in an unevaluated context, it seems that the answer is no.
   3411     if (!isUnevaluatedContext()) {
   3412       QualType CapturedType = getCapturedDeclRefType(cast<VarDecl>(VD), Loc);
   3413       if (!CapturedType.isNull())
   3414         type = CapturedType;
   3415     }
   3416
   3417     break;
   3418   }
  .....
   3516 }


================
Comment at: clang/test/SemaCXX/lambda-default-arg.cpp:6
+  return [=](float b = a) -> bool {
+    return a < 0;
+  }();
----------------
Is the use of `a` in the lambda body intentional? It isn't needed to reproduce the assertion failure, so its presence here is a bit confusing from a test perspective as it implies that variable capture is somehow related. I suggest just returning `true`. Likewise for the other two tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143109