[clang] Correctly handle by-reference capture with an initializer that is a pack expansion in lambdas.

[massberg]

Summary

Ensure that the correct information whether an init-capture of a lambda
is passed by reference or by copy. This information is already computed
and has to be passed to the place where NewInitCaptureType is
created.

Before this fix it has been checked whether the VarDecl is a reference
type. This doesn't work for packed expansions, as the information
whether it is passed by reference or by copy is stored at the pattern of
a PackExpansionType and not at the type itself.

However, as the information has been already computed, we just have to
pass it.

Add test that lambda captures with var decls which are reference types
are created in the AST.

Fixes #49266

[massberg]

Add a diagnostics test for pack expansions to check if args are passed by reference and not by value.

[massberg]

Remove unnecessary code from test.

[ilya-biryukov]

clang/lib/Sema/TreeTransform.h

13152

Could we use C->getCaptureKind() == LCK_ByRef instead?

It seems like that was the intention of the function in the first place (that's was the other callsite from the parser is doing): pass what was written by the user and let the function figure out how to actually build the types.
Normally we want to unify the code that parser uses for non-dependent code and the tree-transforms where possible.

[massberg]

clang/lib/Sema/TreeTransform.h

13152

C-getCaptureKind is equal to LCK_ByCopy, even if the expansion pack is passed by refernece.

[massberg]

clang/lib/Sema/TreeTransform.h

13152

I will check if this is by error and if it should be set to LCK_ByRef in case of references to pack expansions.

[massberg]

Moved check if we have a pack expansion with reference type to Sema::BuildLambdaExpr.

I'm still not sure if this is the right place, or if From.isCopyCapture() should be false for pack expansions with refernece types.

[massberg]

Instead of lokking if there is a pack expansion pass the already computed information whether we have a by ref or a by copy type.

[massberg]

Simplify code and update description.

[massberg]

clang/lib/Sema/TreeTransform.h

13152

I have fixed the code and updated the description.

[ilya-biryukov]

Thanks, LGTM!

clang/include/clang/Sema/Sema.h

7101

NIT 1: naming, this should be in UpperCamelCase
NIT 2: maybe rename to ByRef? It might be a bit confusing to talk about types here as the idea is to actually pass information about the syntax. Types can be inspected by looking at Var.

clang/include/clang/Sema/Sema.h

   ///  variables appropriately.
   VarDecl *createLambdaInitCaptureVarDecl(SourceLocation Loc,
                                           QualType InitCaptureType,
                                           SourceLocation EllipsisLoc,
                                           IdentifierInfo *Id,
                                           unsigned InitStyle, Expr *Init);
 
   /// Add an init-capture to a lambda scope.
-  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var);
+  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var,
+                      bool isReferenceType);
 
   /// Note that we have finished the explicit captures for the
   /// given lambda.
   void finishLambdaExplicitCaptures(sema::LambdaScopeInfo *LSI);
 
   /// \brief This is called after parsing the explicit template parameter list
   /// on a lambda (if it exists) in C++2a.
   void ActOnLambdaExplicitTemplateParameterList(SourceLocation LAngleLoc,

clang/lib/Sema/SemaLambda.cpp

VarDecl *Sema::createLambdaInitCaptureVarDecl(SourceLocation Loc,
   NewVD->setInitStyle(static_cast<VarDecl::InitializationStyle>(InitStyle));
   NewVD->markUsed(Context);
   NewVD->setInit(Init);
   if (NewVD->isParameterPack())
     getCurLambda()->LocalPacks.push_back(NewVD);
   return NewVD;
 }
 
-void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var) {
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var,
+                          bool isReferenceType) {
   assert(Var->isInitCapture() && "init capture flag should be set");
-  LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(),
-                  /*isNested*/false, Var->getLocation(), SourceLocation(),
-                  Var->getType(), /*Invalid*/false);
+  LSI->addCapture(Var, /*isBlock*/ false, isReferenceType,
+                  /*isNested*/ false, Var->getLocation(), SourceLocation(),
+                  Var->getType(), /*Invalid*/ false);
 }
 
 void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
                                         Declarator &ParamInfo,
                                         Scope *CurScope) {
   LambdaScopeInfo *const LSI = getCurLambda();
   assert(LSI && "LambdaScopeInfo should be on stack!");
 
if (C->EllipsisLoc.isValid()) {
 
         // Just ignore the ellipsis.
       }
     } else if (Var->isParameterPack()) {
       ContainsUnexpandedParameterPack = true;
     }
 
     if (C->Init.isUsable()) {
-      addInitCapture(LSI, cast<VarDecl>(Var));
+      addInitCapture(LSI, cast<VarDecl>(Var), C->Kind == LCK_ByRef);
     } else {
       TryCaptureKind Kind = C->Kind == LCK_ByRef ? TryCapture_ExplicitByRef :
                                                    TryCapture_ExplicitByVal;
       tryCaptureVariable(Var, C->Loc, Kind, EllipsisLoc);
     }
     if (!LSI->Captures.empty())
       LSI->ExplicitCaptureRanges[LSI->Captures.size() - 1] = C->ExplicitRange;
   }

clang/lib/Sema/TreeTransform.h

auto SubstInitCapture = [&](SourceLocation EllipsisLoc,
       if (NewExprInitResult.isInvalid()) {
         Result.Expansions.push_back(InitCaptureInfoTy(ExprError(), QualType()));
         return;
       }
       Expr *NewExprInit = NewExprInitResult.get();
 
       QualType NewInitCaptureType =
           getSema().buildLambdaInitCaptureInitialization(
-              C->getLocation(), OldVD->getType()->isReferenceType(),
+              C->getLocation(), C->getCaptureKind() == LCK_ByRef,
               EllipsisLoc, NumExpansions, OldVD->getIdentifier(),
               cast<VarDecl>(C->getCapturedVar())->getInitStyle() !=
                   VarDecl::CInit,
               NewExprInit);
       Result.Expansions.push_back(
           InitCaptureInfoTy(NewExprInit, NewInitCaptureType));
     };
 
if (E->isInitCapture(C)) {
         VarDecl *NewVD = getSema().createLambdaInitCaptureVarDecl(
             OldVD->getLocation(), InitQualType, NewC.EllipsisLoc,
             OldVD->getIdentifier(), OldVD->getInitStyle(), Init.get());
         if (!NewVD) {
           Invalid = true;
           break;
         }
         NewVDs.push_back(NewVD);
-        getSema().addInitCapture(LSI, NewVD);
+        getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
       }
 
       if (Invalid)
         break;
 
       getDerived().transformedLocalDecl(OldVD, NewVDs);
       continue;
     }

clang/test/SemaCXX/lambda-pack-expansion.cpp

+// RUN: %clang_cc1 -std=c++20 -Wno-unused-value -fsyntax-only -verify %s
+
+namespace GH49266 {
+struct X {
+  X() = default;
+  X(X const&) = delete; // expected-note {{'X' has been explicitly marked deleted here}}
+};
+
+void take_by_copy(auto &...args) {
+  [...args = args] {}(); // expected-error {{call to deleted constructor}}
+}
+
+void take_by_ref(auto &...args) {
+  [&...args = args] {}(); // args is passed by reference and not copied.
+}
+
+void foo() {
+  X x;
+  take_by_copy(x); // expected-note {{in instantiation of function template specialization}}
+  take_by_ref(x);
+}
+}

clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

if (!GetParam().isCXX14OrLater()) {
     return;
   }
   EXPECT_FALSE(matches(
       "int main() { auto lambda = [xx = 1] {return xx;}; }",
       lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
           hasName("cc"), hasInitializer(integerLiteral(equals(1))))))))));
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureOfReferenceType) {
+  if (!GetParam().isCXX20OrLater()) {
+    return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+      lambdaCapture(capturesVar(varDecl(hasType(referenceType()))))));
+  EXPECT_TRUE(matches("template <class ...T> void f(T &...args) {"
+                      "  [&...args = args] () mutable {"
+                      "  }();"
+                      "}"
+                      "int main() {"
+                      "  int a;"
+                      "  f(a);"
+                      "}", matcher));
+  EXPECT_FALSE(matches("template <class ...T> void f(T &...args) {"
+                       "  [...args = args] () mutable {"
+                       "  }();"
+                       "}"
+                       "int main() {"
+                       "  int a;"
+                       "  f(a);"
+                       "}", matcher));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageCalees) {
   StatementMatcher MessagingFoo =
       objcMessageExpr(callee(objcMethodDecl(hasName("foo"))));
 
   EXPECT_TRUE(matchesObjC("@interface I"
                           "+ (void)foo;"
                           "@end\n"
                           "int main() {"