Improve checking of explicit captures in a C++11 lambda expression:

- Actually building the var -> capture mapping properly (there was an off-by-one error)
  - Keeping track of the source location of each capture
  - Minor QoI improvements, e.g, highlighing the prior capture if
  there are multiple captures, pointing at the variable declaration we
  found if we reject it.

As part of this, add standard citations for the various semantic
checks we perform, and note where we're not performing those checks as
we should.

llvm-svn: 149462

clang/include/clang/Sema/ScopeInfo.h

@@ -140,15 +140,18 @@ public:
     // copy constructor.
     llvm::PointerIntPair<Expr*, 1, bool> CopyExprAndNested;
 
+    /// \brief The source location at which the first capture occurred..
+    SourceLocation Loc;
+    
   public:
-    Capture(VarDecl *Var, bool isByref, bool isNested, Expr *Cpy)
+    Capture(VarDecl *Var, bool isByref, bool isNested, SourceLocation Loc,
+            Expr *Cpy)
       : VarAndKind(Var, isByref ? Cap_ByRef : Cap_ByVal),
         CopyExprAndNested(Cpy, isNested) {}
 
     enum IsThisCapture { ThisCapture };
-    Capture(IsThisCapture, bool isNested)
-      : VarAndKind(0, Cap_This),
-        CopyExprAndNested(0, isNested) {
+    Capture(IsThisCapture, bool isNested, SourceLocation Loc)
+      : VarAndKind(0, Cap_This), CopyExprAndNested(0, isNested), Loc(Loc) {
     }
 
     bool isThisCapture() const { return VarAndKind.getInt() == Cap_This; }
@@ -160,6 +163,10 @@ public:
     VarDecl *getVariable() const {
       return VarAndKind.getPointer();
     }
+    
+    /// \brief Retrieve the location at which this variable was captured.
+    SourceLocation getLocation() const { return Loc; }
+    
     Expr *getCopyExpr() const {
       return CopyExprAndNested.getPointer();
     }
@@ -188,13 +195,14 @@ public:
   /// or null if unknown.
   QualType ReturnType;
 
-  void AddCapture(VarDecl *Var, bool isByref, bool isNested, Expr *Cpy) {
-    Captures.push_back(Capture(Var, isByref, isNested, Cpy));
+  void AddCapture(VarDecl *Var, bool isByref, bool isNested, SourceLocation Loc,
+                  Expr *Cpy) {
+    Captures.push_back(Capture(Var, isByref, isNested, Loc, Cpy));
     CaptureMap[Var] = Captures.size();
   }
 
-  void AddThisCapture(bool isNested) {
-    Captures.push_back(Capture(Capture::ThisCapture, isNested));
+  void AddThisCapture(bool isNested, SourceLocation Loc) {
+    Captures.push_back(Capture(Capture::ThisCapture, isNested, Loc));
     CXXThisCaptureIndex = Captures.size();
   }
 

clang/lib/Sema/SemaExpr.cpp

@@ -1263,7 +1263,8 @@ static CaptureResult propagateCapture(Sema &S, unsigned ValidScopeIndex,
          i != e; ++i) {
     BlockScopeInfo *innerBlock = cast<BlockScopeInfo>(S.FunctionScopes[i]);
     innerBlock->AddCapture(Cap.getVariable(), Cap.isReferenceCapture(),
-                           /*nested*/ true, Cap.getCopyExpr());
+                           /*nested*/ true, Cap.getLocation(),
+                           Cap.getCopyExpr());
   }
 
   return Cap.isReferenceCapture() ? CR_CaptureByRef : CR_Capture;
@@ -1377,7 +1378,7 @@ static CaptureResult shouldCaptureValueReference(Sema &S, SourceLocation loc,
     cast<BlockScopeInfo>(S.FunctionScopes[functionScopesIndex]);
 
   // Build a valid capture in this scope.
-  blockScope->AddCapture(var, byRef, /*nested*/ false, copyExpr);
+  blockScope->AddCapture(var, byRef, /*nested*/ false, loc, copyExpr);
 
   // Propagate that to inner captures if necessary.
   return propagateCapture(S, functionScopesIndex,

clang/lib/Sema/SemaExprCXX.cpp

@@ -706,7 +706,7 @@ void Sema::CheckCXXThisCapture(SourceLocation Loc) {
        NumClosures; --idx, --NumClosures) {
     CapturingScopeInfo *CSI = cast<CapturingScopeInfo>(FunctionScopes[idx]);
     bool isNested = NumClosures > 1;
-    CSI->AddThisCapture(isNested);
+    CSI->AddThisCapture(isNested, Loc);
   }
 }
 
@@ -4869,16 +4869,27 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
   for (llvm::SmallVector<LambdaCapture, 4>::const_iterator
        C = Intro.Captures.begin(), E = Intro.Captures.end(); C != E; ++C) {
     if (C->Kind == LCK_This) {
+      // C++11 [expr.prim.lambda]p8:
+      //   An identifier or this shall not appear more than once in a 
+      //   lambda-capture.
       if (!ThisCaptureType.isNull()) {
-        Diag(C->Loc, diag::err_capture_more_than_once) << "'this'";
+        Diag(C->Loc, diag::err_capture_more_than_once) 
+          << "'this'"
+          << SourceRange(Captures[CXXThisCaptureIndex].getLocation());
         continue;
       }
 
+      // C++11 [expr.prim.lambda]p8:
+      //   If a lambda-capture includes a capture-default that is =, the 
+      //   lambda-capture shall not contain this [...].
       if (Intro.Default == LCD_ByCopy) {
         Diag(C->Loc, diag::err_this_capture_with_copy_default);
         continue;
       }
 
+      // C++11 [expr.prim.lambda]p12:
+      //   If this is captured by a local lambda expression, its nearest
+      //   enclosing function shall be a non-static member function.
       ThisCaptureType = getCurrentThisType();
       if (ThisCaptureType.isNull()) {
         Diag(C->Loc, diag::err_invalid_this_use);
@@ -4888,15 +4899,20 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
 
       // FIXME: Need getCurCapture().
       bool isNested = getCurBlock() || getCurLambda();
-      CapturingScopeInfo::Capture Cap(CapturingScopeInfo::Capture::ThisCapture,
-                                      isNested);
-      Captures.push_back(Cap);
       CXXThisCaptureIndex = Captures.size();
+      CapturingScopeInfo::Capture Cap(CapturingScopeInfo::Capture::ThisCapture,
+                                      isNested, C->Loc);
+      Captures.push_back(Cap);
       continue;
     }
 
     assert(C->Id && "missing identifier for capture");
 
+    // C++11 [expr.prim.lambda]p8:
+    //   If a lambda-capture includes a capture-default that is &, the 
+    //   identifiers in the lambda-capture shall not be preceded by &.
+    //   If a lambda-capture includes a capture-default that is =, [...]
+    //   each identifier it contains shall be preceded by &.
     if (C->Kind == LCK_ByRef && Intro.Default == LCD_ByRef) {
       Diag(C->Loc, diag::err_reference_capture_with_reference_default);
       continue;
@@ -4907,43 +4923,56 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
 
     DeclarationNameInfo Name(C->Id, C->Loc);
     LookupResult R(*this, Name, LookupOrdinaryName);
-    CXXScopeSpec ScopeSpec;
-    LookupParsedName(R, CurScope, &ScopeSpec);
+    LookupName(R, CurScope);
     if (R.isAmbiguous())
       continue;
     if (R.empty()) {
+      // FIXME: Disable corrections that would add qualification?
+      CXXScopeSpec ScopeSpec;
       DeclFilterCCC<VarDecl> Validator;
       if (DiagnoseEmptyLookup(CurScope, ScopeSpec, R, Validator))
         continue;
     }
 
+    // C++11 [expr.prim.lambda]p10:
+    //   The identifiers in a capture-list are looked up using the usual rules
+    //   for unqualified name lookup (3.4.1); each such lookup shall find a 
+    //   variable with automatic storage duration declared in the reaching 
+    //   scope of the local lambda expression.
+    // FIXME: Check reaching scope.
     VarDecl *Var = R.getAsSingle<VarDecl>();
     if (!Var) {
       Diag(C->Loc, diag::err_capture_does_not_name_variable) << C->Id;
       continue;
     }
 
-    if (CaptureMap.count(Var)) {
-      Diag(C->Loc, diag::err_capture_more_than_once) << C->Id;
-      continue;
-    }
-
     if (!Var->hasLocalStorage()) {
       Diag(C->Loc, diag::err_capture_non_automatic_variable) << C->Id;
+      Diag(Var->getLocation(), diag::note_previous_decl) << C->Id;
       continue;
     }
-
+    
     if (Var->hasAttr<BlocksAttr>()) {
       Diag(C->Loc, diag::err_lambda_capture_block) << C->Id;
       Diag(Var->getLocation(), diag::note_previous_decl) << C->Id;
       continue;
     }
+
+    // C++11 [expr.prim.lambda]p8:
+    //   An identifier or this shall not appear more than once in a 
+    //   lambda-capture.
+    if (CaptureMap.count(Var)) {
+      Diag(C->Loc, diag::err_capture_more_than_once) 
+        << C->Id
+        << SourceRange(Captures[CaptureMap[Var]].getLocation());
+      continue;
+    }
     
     // FIXME: If this is capture by copy, make sure that we can in fact copy
     // the variable.
-    Captures.push_back(LambdaScopeInfo::Capture(Var, C->Kind == LCK_ByRef,
-                                                /*isNested*/false, 0));
     CaptureMap[Var] = Captures.size();
+    Captures.push_back(LambdaScopeInfo::Capture(Var, C->Kind == LCK_ByRef,
+                                                /*isNested*/false, C->Loc, 0));
   }
 
   // Build the call operator; we don't really have all the relevant information
@@ -4951,6 +4980,9 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
   QualType MethodTy;
   TypeSourceInfo *MethodTyInfo;
   if (ParamInfo.getNumTypeObjects() == 0) {
+    // C++11 [expr.prim.lambda]p4:
+    //   If a lambda-expression does not include a lambda-declarator, it is as 
+    //   if the lambda-declarator were ().
     FunctionProtoType::ExtProtoInfo EPI;
     EPI.TypeQuals |= DeclSpec::TQ_const;
     MethodTy = Context.getFunctionType(Context.DependentTy,
@@ -4960,6 +4992,11 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
     assert(ParamInfo.isFunctionDeclarator() &&
            "lambda-declarator is a function");
     DeclaratorChunk::FunctionTypeInfo &FTI = ParamInfo.getFunctionTypeInfo();
+    
+    // C++11 [expr.prim.lambda]p5:
+    //   This function call operator is declared const (9.3.1) if and only if 
+    //   the lambda- expression’s parameter-declaration-clause is not followed 
+    //   by mutable. It is neither virtual nor declared volatile.
     if (!FTI.hasMutableQualifier())
       FTI.TypeQuals |= DeclSpec::TQ_const;
     MethodTyInfo = GetTypeForDeclarator(ParamInfo, CurScope);
@@ -4969,6 +5006,11 @@ void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro,
     assert(!MethodTy.isNull() && "no type from lambda declarator");
   }
 
+  // C++11 [expr.prim.lambda]p5:
+  //   The closure type for a lambda-expression has a public inline function 
+  //   call operator (13.5.4) whose parameters and return type are described by
+  //   the lambda-expression’s parameter-declaration-clause and 
+  //   trailing-return-type respectively.
   DeclarationName MethodName
     = Context.DeclarationNames.getCXXOperatorName(OO_Call);
   CXXMethodDecl *Method

clang/lib/Sema/SemaExprObjC.cpp

@@ -265,7 +265,8 @@ ObjCMethodDecl *Sema::tryCaptureObjCSelf() {
     if (captureIndex) break;
 
     bool nested = isa<BlockScopeInfo>(FunctionScopes[idx-1]);
-    blockScope->AddCapture(self, /*byref*/ false, nested, /*copy*/ 0);
+    blockScope->AddCapture(self, /*byref*/ false, nested, self->getLocation(),
+                           /*copy*/ 0);
     captureIndex = blockScope->Captures.size(); // +1
   }
 

clang/test/CXX/expr/expr.prim/expr.prim.lambda/p10.cpp

@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -std=c++11 %s -verify
+
+int GlobalVar; // expected-note 2{{declared here}}
+
+namespace N {
+  int AmbiguousVar; // expected-note {{candidate}}
+}
+int AmbiguousVar; // expected-note {{candidate}}
+using namespace N;
+
+class X0 {
+  int Member;
+
+  static void Overload(int);
+  void Overload();
+  virtual X0& Overload(float);
+
+  void explicit_capture() {
+    [&Overload] () {}; // expected-error {{does not name a variable}} expected-error {{not supported yet}}
+    [&GlobalVar] () {}; // expected-error {{does not have automatic storage duration}} expected-error {{not supported yet}}
+    [&AmbiguousVar] () {} // expected-error {{reference to 'AmbiguousVar' is ambiguous}} expected-error {{not supported yet}}
+    [&Globalvar] () {}; // expected-error {{use of undeclared identifier 'Globalvar'; did you mean 'GlobalVar}}
+  }
+};

clang/test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp

@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++11 %s -verify
+
+class X0 {
+  void explicit_capture() {
+    int foo;
+
+    [foo, foo] () {}; // expected-error {{'foo' can appear only once}} expected-error {{not supported yet}}
+    [this, this] () {}; // expected-error {{'this' can appear only once}} expected-error {{not supported yet}}
+    [=, foo] () {}; // expected-error {{'&' must precede a capture when}} expected-error {{not supported yet}}
+    [=, &foo] () {}; // expected-error {{not supported yet}}
+    [=, this] () {}; // expected-error {{'this' cannot appear}} expected-error {{not supported yet}}
+    [&, foo] () {}; // expected-error {{not supported yet}}
+    [&, &foo] () {}; // expected-error {{'&' cannot precede a capture when}} expected-error {{not supported yet}}
+    [&, this] () {}; // expected-error {{not supported yet}}
+  }
+};

clang/test/SemaCXX/lambda-expressions.cpp

@@ -3,14 +3,6 @@
 namespace std { class type_info; };
 
 namespace ExplicitCapture {
-  int GlobalVar; // expected-note {{declared here}}
-
-  namespace N {
-    int AmbiguousVar; // expected-note {{candidate}}
-  }
-  int AmbiguousVar; // expected-note {{candidate}}
-  using namespace N;
-
   class C {
     int Member;
 
@@ -18,28 +10,12 @@ namespace ExplicitCapture {
     void Overload();
     virtual C& Overload(float);
 
-    void ExplicitCapture() {
-      int foo;
-
-      [foo, foo] () {}; // expected-error {{'foo' can appear only once}} expected-error {{not supported yet}}
-      [this, this] () {}; // expected-error {{'this' can appear only once}} expected-error {{not supported yet}}
-      [=, foo] () {}; // expected-error {{'&' must precede a capture when}} expected-error {{not supported yet}}
-      [=, &foo] () {}; // expected-error {{not supported yet}}
-      [=, this] () {}; // expected-error {{'this' cannot appear}} expected-error {{not supported yet}}
-      [&, foo] () {}; // expected-error {{not supported yet}}
-      [&, &foo] () {}; // expected-error {{'&' cannot precede a capture when}} expected-error {{not supported yet}}
-      [&, this] () {}; // expected-error {{not supported yet}}
-      [&Overload] () {}; // expected-error {{does not name a variable}} expected-error {{not supported yet}}
-      [&GlobalVar] () {}; // expected-error {{does not have automatic storage duration}} expected-error {{not supported yet}}
-      [&AmbiguousVar] () {} // expected-error {{reference to 'AmbiguousVar' is ambiguous}} expected-error {{not supported yet}}
-      [&Globalvar] () {}; // expected-error {{use of undeclared identifier 'Globalvar'; did you mean 'GlobalVar}}
-    }
-
     void ImplicitThisCapture() {
       [](){(void)Member;}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-error {{not supported yet}}
       [&](){(void)Member;}; // expected-error {{not supported yet}}
-      [this](){(void)Member;}; // expected-error {{not supported yet}}
-      [this]{[this]{};};// expected-error 2 {{not supported yet}}
+      // FIXME: 'this' captures below don't actually work yet
+      // FIXME: [this](){(void)Member;};
+      // FIXME: [this]{[this]{};};
       []{[this]{};};// expected-error {{'this' cannot be implicitly captured in this context}} expected-error 2 {{not supported yet}}
       []{Overload(3);}; // expected-error {{not supported yet}}
       []{Overload();}; // expected-error {{'this' cannot be implicitly captured in this context}} expected-error {{not supported yet}}