[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

13153

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

13153

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

[massberg]

clang/lib/Sema/TreeTransform.h

13153

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

13153

I have fixed the code and updated the description.

[ilya-biryukov]

Thanks, LGTM!

clang/include/clang/Sema/Sema.h

7101 ↗

(On Diff #480800)

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/lib/Sema/SemaLambda.cpp

for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I) {
                                From.isCopyCapture() ? LCK_StarThis : LCK_This);
         } else if (From.isVLATypeCapture()) {
           return LambdaCapture(From.getLocation(), IsImplicit, LCK_VLAType);
         } else {
           assert(From.isVariableCapture() && "unknown kind of capture");
           ValueDecl *Var = From.getVariable();
           LambdaCaptureKind Kind =
               From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef;
+          if (isa<PackExpansionType>(Var->getType()) &&
+              cast<PackExpansionType>(Var->getType())
+                                        ->getPattern()
+              ->isReferenceType()) {
+            Kind = LCK_ByRef;
+          }
           return LambdaCapture(From.getLocation(), IsImplicit, Kind, Var,
                                From.getEllipsisLoc());
         }
       }();
 
       // Form the initializer for the capture field.
       ExprResult Init = BuildCaptureInit(From, ImplicitCaptureLoc);
 

clang/lib/Sema/TreeTransform.h

auto SubstInitCapture = [&](SourceLocation EllipsisLoc,
           OldVD->getInit(), OldVD->getInitStyle() == VarDecl::CallInit);
 
       if (NewExprInitResult.isInvalid()) {
         Result.Expansions.push_back(InitCaptureInfoTy(ExprError(), QualType()));
         return;
       }
       Expr *NewExprInit = NewExprInitResult.get();
 
+      bool isReferenceType = (C->getCaptureKind() == LCK_ByRef);
       QualType NewInitCaptureType =
           getSema().buildLambdaInitCaptureInitialization(
-              C->getLocation(), OldVD->getType()->isReferenceType(),
+              C->getLocation(), isReferenceType,
               EllipsisLoc, NumExpansions, OldVD->getIdentifier(),
               cast<VarDecl>(C->getCapturedVar())->getInitStyle() !=
                   VarDecl::CInit,
               NewExprInit);
       Result.Expansions.push_back(
           InitCaptureInfoTy(NewExprInit, NewInitCaptureType));
     };
 

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() {"