<div class="gmail_quote">On Mon, Mar 12, 2012 at 5:37 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rjmccall<br>
Date: Mon Mar 12 19:37:01 2012<br>
New Revision: 152593<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=152593&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=152593&view=rev</a><br>
Log:<br>
Alternate fix to PR12248: put Sema in charge of special-casing<br>
the diagnostic for assigning to a copied block capture. This has<br>
the pleasant side-effect of letting us special-case the diagnostic<br>
for assigning to a copied lambda capture as well, without introducing<br>
a new non-modifiable enumerator for it.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/AST/Expr.h<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/AST/ExprClassification.cpp<br>
cfe/trunk/lib/Sema/SemaExpr.cpp<br>
cfe/trunk/test/SemaCXX/lambda-expressions.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Expr.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=152593&r1=152592&r2=152593&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=152593&r1=152592&r2=152593&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/AST/Expr.h (original)<br>
+++ cfe/trunk/include/clang/AST/Expr.h Mon Mar 12 19:37:01 2012<br>
@@ -236,7 +236,6 @@<br>
MLV_IncompleteType,<br>
MLV_ConstQualified,<br>
MLV_ArrayType,<br>
- MLV_NotBlockQualified,<br>
MLV_ReadonlyProperty,<br>
MLV_NoSetterProperty,<br>
MLV_MemberFunction,<br>
@@ -272,7 +271,6 @@<br>
CM_RValue, // Not modifiable because it's an rvalue<br>
CM_Function, // Not modifiable because it's a function; C++ only<br>
CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext<br>
- CM_NotBlockQualified, // Not captured in the closure<br>
CM_NoSetterProperty,// Implicit assignment to ObjC property without setter<br>
CM_ConstQualified,<br>
CM_ArrayType,<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=152593&r1=152592&r2=152593&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=152593&r1=152592&r2=152593&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Mar 12 19:37:01 2012<br>
@@ -4387,6 +4387,8 @@<br>
"vector is not assignable (contains duplicate components)">;<br>
def err_block_decl_ref_not_modifiable_lvalue : Error<<br>
"variable is not assignable (missing __block type specifier)">;<br>
+def err_lambda_decl_ref_not_modifiable_lvalue : Error<<br>
+ "variable is not assignable (captured by copy)">;<br>
def err_typecheck_call_not_function : Error<<br>
"called object type %0 is not a function or function pointer">;<br>
def err_call_incomplete_return : Error<<br>
<br>
Modified: cfe/trunk/lib/AST/ExprClassification.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprClassification.cpp?rev=152593&r1=152592&r2=152593&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprClassification.cpp?rev=152593&r1=152592&r2=152593&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ExprClassification.cpp (original)<br>
+++ cfe/trunk/lib/AST/ExprClassification.cpp Mon Mar 12 19:37:01 2012<br>
@@ -567,18 +567,8 @@<br>
<br>
CanQualType CT = Ctx.getCanonicalType(E->getType());<br>
// Const stuff is obviously not modifiable.<br>
- if (CT.isConstQualified()) {<br>
- // Special-case variables captured by blocks to get an improved<br>
- // diagnostic.<br>
- if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {<br>
- if (DRE->refersToEnclosingLocal() &&<br>
- isa<VarDecl>(DRE->getDecl()) &&<br>
- cast<VarDecl>(DRE->getDecl())->hasLocalStorage() &&<br>
- !DRE->getDecl()->hasAttr<BlocksAttr>())<br>
- return Cl::CM_NotBlockQualified;<br>
- }<br>
+ if (CT.isConstQualified())<br>
return Cl::CM_ConstQualified;<br>
- }<br>
<br>
// Arrays are not modifiable, only their elements are.<br>
if (CT->isArrayType())<br>
@@ -645,7 +635,6 @@<br>
case Cl::CM_Function: return MLV_NotObjectType;<br>
case Cl::CM_LValueCast:<br>
llvm_unreachable("CM_LValueCast and CL_LValue don't match");<br>
- case Cl::CM_NotBlockQualified: return MLV_NotBlockQualified;<br>
case Cl::CM_NoSetterProperty: return MLV_NoSetterProperty;<br>
case Cl::CM_ConstQualified: return MLV_ConstQualified;<br>
case Cl::CM_ArrayType: return MLV_ArrayType;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=152593&r1=152592&r2=152593&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=152593&r1=152592&r2=152593&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Mar 12 19:37:01 2012<br>
@@ -7127,6 +7127,32 @@<br>
return Base->getMethodDecl() != 0;<br>
}<br>
<br>
+/// Is the given expression (which must be 'const') a reference to a<br>
+/// variable which was originally non-const, but which has become<br>
+/// 'const' due to being captured within a block?<br>
+enum NonConstCaptureKind { NCCK_None, NCCK_Block, NCCK_Lambda };<br>
+static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {<br>
+ assert(E->isLValue() && E->getType().isConstQualified());<br>
+ E = E->IgnoreParens();<br>
+<br>
+ // Must be a reference to a declaration from an enclosing scope.<br>
+ DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E);<br>
+ if (!DRE) return NCCK_None;<br>
+ if (!DRE->refersToEnclosingLocal()) return NCCK_None;<br>
+<br>
+ // The declaration must be a variable which is not declared 'const'.<br>
+ VarDecl *var = dyn_cast<VarDecl>(DRE->getDecl());<br>
+ if (!var) return NCCK_None;<br>
+ if (var->getType().isConstQualified()) return NCCK_None;<br>
+ assert(var->hasLocalStorage() && "capture added 'const' to non-local?");<br>
+<br>
+ // Decide whether the first capture was for a block or a lambda.<br>
+ DeclContext *DC = S.CurContext;<br>
+ while (DC->getParent() != var->getDeclContext())<br>
+ DC = DC->getParent();<br>
+ return (isa<BlockDecl>(DC) ? NCCK_Block : NCCK_Lambda);<br>
+}<br>
+<br>
/// CheckForModifiableLvalue - Verify that E is a modifiable lvalue. If not,<br>
/// emit an error and return true. If so, return false.<br>
static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {<br>
@@ -7148,6 +7174,16 @@<br>
case Expr::MLV_ConstQualified:<br>
Diag = diag::err_typecheck_assign_const;<br>
<br>
+ // Use a specialized diagnostic when we're assigning to an object<br>
+ // from an enclosing function or block.<br>
+ if (NonConstCaptureKind NCCK = isReferenceToNonConstCapture(S, E)) {<br>
+ if (NCCK == NCCK_Block)<br>
+ Diag = diag::err_block_decl_ref_not_modifiable_lvalue;<br>
+ else<br>
+ Diag = diag::err_lambda_decl_ref_not_modifiable_lvalue;<br>
+ break;<br>
+ }<br>
+<br>
// In ARC, use some specialized diagnostics for occasions where we<br>
// infer 'const'. These are always pseudo-strong variables.<br>
if (S.getLangOpts().ObjCAutoRefCount) {<br>
@@ -7210,9 +7246,6 @@<br>
case Expr::MLV_DuplicateVectorComponents:<br>
Diag = diag::err_typecheck_duplicate_vector_components_not_mlvalue;<br>
break;<br>
- case Expr::MLV_NotBlockQualified:<br>
- Diag = diag::err_block_decl_ref_not_modifiable_lvalue;<br>
- break;<br>
case Expr::MLV_ReadonlyProperty:<br>
case Expr::MLV_NoSetterProperty:<br>
llvm_unreachable("readonly properties should be processed differently");<br>
<br>
Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=152593&r1=152592&r2=152593&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=152593&r1=152592&r2=152593&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Mon Mar 12 19:37:01 2012<br>
@@ -139,3 +139,12 @@<br>
unsigned int result = 0;<br>
auto l = [&]() { ++result; };<br>
}<br>
+<br>
+namespace ModifyingCapture {<br>
+ void test() {<br>
+ int n = 0;<br>
+ [=] {<br>
+ n = 1; // expected-error {{variable is not assignable (captured by copy)}}<br></blockquote><div><br></div><div>It would be great if this diagnostic mentioned that the lambda is not marked as 'mutable'.</div>
</div>