Skip to content

Commit af7d76c

Browse files
WeverythingWeverything
committedApr 11, 2015
Improve the error message for assigning to read-only variables.
Previously, many error messages would simply be "read-only variable is not assignable" This change provides more information about why the variable is not assignable, as well as note to where the const is located. Differential Revision: http://reviews.llvm.org/D4479 llvm-svn: 234677
1 parent d9a21bf commit af7d76c

14 files changed

+563
-26
lines changed
 

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

+17-1
Original file line numberDiff line numberDiff line change
@@ -5030,7 +5030,23 @@ def err_typecheck_op_on_nonoverlapping_address_space_pointers : Error<
50305030
"%select{comparison between %diff{ ($ and $)|}0,1"
50315031
"|arithmetic operation with operands of type %diff{ ($ and $)|}0,1}2"
50325032
" which are pointers to non-overlapping address spaces">;
5033-
def err_typecheck_assign_const : Error<"read-only variable is not assignable">;
5033+
5034+
def err_typecheck_assign_const : Error<
5035+
"%select{"
5036+
"cannot assign to return value because function %1 returns a const value|"
5037+
"cannot assign to variable %1 with const-qualified type %2|"
5038+
"cannot assign to %select{non-|}1static data member %2 "
5039+
"with const-qualified type %3|"
5040+
"cannot assign to non-static data member within const member function %1|"
5041+
"read-only variable is not assignable}0">;
5042+
5043+
def note_typecheck_assign_const : Note<
5044+
"%select{"
5045+
"function %1 which returns const-qualified type %2 declared here|"
5046+
"variable %1 declared const here|"
5047+
"%select{non-|}1static data member %2 declared const here|"
5048+
"member function %q1 is declared const here}0">;
5049+
50345050
def warn_mixed_sign_comparison : Warning<
50355051
"comparison of integers of different signs: %0 and %1">,
50365052
InGroup<SignCompare>, DefaultIgnore;

‎clang/lib/Sema/SemaExpr.cpp

+141-3
Original file line numberDiff line numberDiff line change
@@ -8982,6 +8982,139 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {
89828982
return (isa<BlockDecl>(DC) ? NCCK_Block : NCCK_Lambda);
89838983
}
89848984

8985+
static bool IsTypeModifiable(QualType Ty, bool IsDereference) {
8986+
Ty = Ty.getNonReferenceType();
8987+
if (IsDereference && Ty->isPointerType())
8988+
Ty = Ty->getPointeeType();
8989+
return !Ty.isConstQualified();
8990+
}
8991+
8992+
/// Emit the "read-only variable not assignable" error and print notes to give
8993+
/// more information about why the variable is not assignable, such as pointing
8994+
/// to the declaration of a const variable, showing that a method is const, or
8995+
/// that the function is returning a const reference.
8996+
static void DiagnoseConstAssignment(Sema &S, const Expr *E,
8997+
SourceLocation Loc) {
8998+
// Update err_typecheck_assign_const and note_typecheck_assign_const
8999+
// when this enum is changed.
9000+
enum {
9001+
ConstFunction,
9002+
ConstVariable,
9003+
ConstMember,
9004+
ConstMethod,
9005+
ConstUnknown, // Keep as last element
9006+
};
9007+
9008+
SourceRange ExprRange = E->getSourceRange();
9009+
9010+
// Only emit one error on the first const found. All other consts will emit
9011+
// a note to the error.
9012+
bool DiagnosticEmitted = false;
9013+
9014+
// Track if the current expression is the result of a derefence, and if the
9015+
// next checked expression is the result of a derefence.
9016+
bool IsDereference = false;
9017+
bool NextIsDereference = false;
9018+
9019+
// Loop to process MemberExpr chains.
9020+
while (true) {
9021+
IsDereference = NextIsDereference;
9022+
NextIsDereference = false;
9023+
9024+
E = E->IgnoreParenImpCasts();
9025+
if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
9026+
NextIsDereference = ME->isArrow();
9027+
const ValueDecl *VD = ME->getMemberDecl();
9028+
if (const FieldDecl *Field = dyn_cast<FieldDecl>(VD)) {
9029+
// Mutable fields can be modified even if the class is const.
9030+
if (Field->isMutable()) {
9031+
assert(DiagnosticEmitted && "Expected diagnostic not emitted.");
9032+
break;
9033+
}
9034+
9035+
if (!IsTypeModifiable(Field->getType(), IsDereference)) {
9036+
if (!DiagnosticEmitted) {
9037+
S.Diag(Loc, diag::err_typecheck_assign_const)
9038+
<< ExprRange << ConstMember << false /*static*/ << Field
9039+
<< Field->getType();
9040+
DiagnosticEmitted = true;
9041+
}
9042+
S.Diag(VD->getLocation(), diag::note_typecheck_assign_const)
9043+
<< ConstMember << false /*static*/ << Field << Field->getType()
9044+
<< Field->getSourceRange();
9045+
}
9046+
E = ME->getBase();
9047+
continue;
9048+
} else if (const VarDecl *VDecl = dyn_cast<VarDecl>(VD)) {
9049+
if (VDecl->getType().isConstQualified()) {
9050+
if (!DiagnosticEmitted) {
9051+
S.Diag(Loc, diag::err_typecheck_assign_const)
9052+
<< ExprRange << ConstMember << true /*static*/ << VDecl
9053+
<< VDecl->getType();
9054+
DiagnosticEmitted = true;
9055+
}
9056+
S.Diag(VD->getLocation(), diag::note_typecheck_assign_const)
9057+
<< ConstMember << true /*static*/ << VDecl << VDecl->getType()
9058+
<< VDecl->getSourceRange();
9059+
}
9060+
// Static fields do not inherit constness from parents.
9061+
break;
9062+
}
9063+
break;
9064+
} // End MemberExpr
9065+
break;
9066+
}
9067+
9068+
if (const CallExpr *CE = dyn_cast<CallExpr>(E)) {
9069+
// Function calls
9070+
const FunctionDecl *FD = CE->getDirectCallee();
9071+
if (!IsTypeModifiable(FD->getReturnType(), IsDereference)) {
9072+
if (!DiagnosticEmitted) {
9073+
S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange
9074+
<< ConstFunction << FD;
9075+
DiagnosticEmitted = true;
9076+
}
9077+
S.Diag(FD->getReturnTypeSourceRange().getBegin(),
9078+
diag::note_typecheck_assign_const)
9079+
<< ConstFunction << FD << FD->getReturnType()
9080+
<< FD->getReturnTypeSourceRange();
9081+
}
9082+
} else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {
9083+
// Point to variable declaration.
9084+
if (const ValueDecl *VD = DRE->getDecl()) {
9085+
if (!IsTypeModifiable(VD->getType(), IsDereference)) {
9086+
if (!DiagnosticEmitted) {
9087+
S.Diag(Loc, diag::err_typecheck_assign_const)
9088+
<< ExprRange << ConstVariable << VD << VD->getType();
9089+
DiagnosticEmitted = true;
9090+
}
9091+
S.Diag(VD->getLocation(), diag::note_typecheck_assign_const)
9092+
<< ConstVariable << VD << VD->getType() << VD->getSourceRange();
9093+
}
9094+
}
9095+
} else if (isa<CXXThisExpr>(E)) {
9096+
if (const DeclContext *DC = S.getFunctionLevelDeclContext()) {
9097+
if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(DC)) {
9098+
if (MD->isConst()) {
9099+
if (!DiagnosticEmitted) {
9100+
S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange
9101+
<< ConstMethod << MD;
9102+
DiagnosticEmitted = true;
9103+
}
9104+
S.Diag(MD->getLocation(), diag::note_typecheck_assign_const)
9105+
<< ConstMethod << MD << MD->getSourceRange();
9106+
}
9107+
}
9108+
}
9109+
}
9110+
9111+
if (DiagnosticEmitted)
9112+
return;
9113+
9114+
// Can't determine a more specific message, so display the generic error.
9115+
S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange << ConstUnknown;
9116+
}
9117+
89859118
/// CheckForModifiableLvalue - Verify that E is a modifiable lvalue. If not,
89869119
/// emit an error and return true. If so, return false.
89879120
static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
@@ -8998,8 +9131,6 @@ static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
89989131
bool NeedType = false;
89999132
switch (IsLV) { // C99 6.5.16p2
90009133
case Expr::MLV_ConstQualified:
9001-
DiagID = diag::err_typecheck_assign_const;
9002-
90039134
// Use a specialized diagnostic when we're assigning to an object
90049135
// from an enclosing function or block.
90059136
if (NonConstCaptureKind NCCK = isReferenceToNonConstCapture(S, E)) {
@@ -9038,13 +9169,20 @@ static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
90389169
if (Loc != OrigLoc)
90399170
Assign = SourceRange(OrigLoc, OrigLoc);
90409171
S.Diag(Loc, DiagID) << E->getSourceRange() << Assign;
9041-
// We need to preserve the AST regardless, so migration tool
9172+
// We need to preserve the AST regardless, so migration tool
90429173
// can do its job.
90439174
return false;
90449175
}
90459176
}
90469177
}
90479178

9179+
// If none of the special cases above are triggered, then this is a
9180+
// simple const assignment.
9181+
if (DiagID == 0) {
9182+
DiagnoseConstAssignment(S, E, Loc);
9183+
return true;
9184+
}
9185+
90489186
break;
90499187
case Expr::MLV_ArrayType:
90509188
case Expr::MLV_ArrayTemporary:

‎clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ template double Foo::As2();
5555
// Partial ordering with conversion function templates.
5656
struct X0 {
5757
template<typename T> operator T*() {
58-
T x = 1;
59-
x = 17; // expected-error{{read-only variable is not assignable}}
58+
T x = 1; // expected-note{{variable 'x' declared const here}}
59+
x = 17; // expected-error{{cannot assign to variable 'x' with const-qualified type 'const int'}}
6060
}
6161

6262
template<typename T> operator T*() const; // expected-note{{explicit instantiation refers here}}

‎clang/test/Sema/anonymous-struct-union.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,17 @@ struct X {
2222
};
2323

2424
void test_unqual_references(struct X x, const struct X xc) {
25+
// expected-note@-1 3{{variable 'xc' declared const here}}
2526
x.i = 0;
2627
x.f = 0.0;
2728
x.f2 = x.f;
2829
x.d = x.f;
2930
x.f3 = 0; // expected-error{{no member named 'f3'}}
3031
x.a = 0;
3132

32-
xc.d = 0.0; // expected-error{{read-only variable is not assignable}}
33-
xc.f = 0; // expected-error{{read-only variable is not assignable}}
34-
xc.a = 0; // expected-error{{read-only variable is not assignable}}
33+
xc.d = 0.0; // expected-error{{cannot assign to variable 'xc' with const-qualified type 'const struct X'}}
34+
xc.f = 0; // expected-error{{cannot assign to variable 'xc' with const-qualified type 'const struct X'}}
35+
xc.a = 0; // expected-error{{cannot assign to variable 'xc' with const-qualified type 'const struct X'}}
3536
}
3637

3738

‎clang/test/Sema/assign.c

+4-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
void *test1(void) { return 0; }
44

55
void test2 (const struct {int a;} *x) {
6-
x->a = 10; // expected-error {{read-only variable is not assignable}}
6+
// expected-note@-1 {{variable 'x' declared const here}}
7+
8+
x->a = 10;
9+
// expected-error-re@-1 {{cannot assign to variable 'x' with const-qualified type 'const struct (anonymous struct at {{.*}}assign.c:5:19) *'}}
710
}
811

912
typedef int arr[10];

‎clang/test/Sema/block-misc.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ void test17() {
184184
}
185185

186186
void test18() {
187-
void (^const blockA)(void) = ^{ };
188-
blockA = ^{ }; // expected-error {{read-only variable is not assignable}}
187+
void (^const blockA)(void) = ^{ }; // expected-note {{variable 'blockA' declared const here}}
188+
blockA = ^{ }; // expected-error {{cannot assign to variable 'blockA' with const-qualified type 'void (^const)(void)}}
189189
}
190190

191191
// rdar://7072507

‎clang/test/SemaCXX/anonymous-union.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ void X::test_unqual_references() {
3939
a = 0;
4040
}
4141

42-
void X::test_unqual_references_const() const {
42+
void X::test_unqual_references_const() const { // expected-note 2{{member function 'X::test_unqual_references_const' is declared const here}}
4343
d = 0.0;
44-
f2 = 0; // expected-error{{read-only variable is not assignable}}
45-
a = 0; // expected-error{{read-only variable is not assignable}}
44+
f2 = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
45+
a = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
4646
}
4747

4848
void test_unqual_references(X x, const X xc) {
49+
// expected-note@-1 2{{variable 'xc' declared const here}}
4950
x.i = 0;
5051
x.f = 0.0;
5152
x.f2 = x.f;
@@ -54,8 +55,8 @@ void test_unqual_references(X x, const X xc) {
5455
x.a = 0;
5556

5657
xc.d = 0.0;
57-
xc.f = 0; // expected-error{{read-only variable is not assignable}}
58-
xc.a = 0; // expected-error{{read-only variable is not assignable}}
58+
xc.f = 0; // expected-error{{cannot assign to variable 'xc' with const-qualified type 'const X'}}
59+
xc.a = 0; // expected-error{{cannot assign to variable 'xc' with const-qualified type 'const X'}}
5960
}
6061

6162

‎clang/test/SemaCXX/captured-statements.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ void test_capture_var() {
8181
}
8282

8383
template <typename S, typename T>
84-
S template_capture_var(S x, T y) {
84+
S template_capture_var(S x, T y) { // expected-note{{variable 'y' declared const here}}
8585
#pragma clang _debug captured
8686
{
8787
x++;
88-
y++; // expected-error{{read-only variable is not assignable}}
88+
y++; // expected-error{{cannot assign to variable 'y' with const-qualified type 'const int'}}
8989
}
9090

9191
return x;

‎clang/test/SemaCXX/cxx0x-constexpr-const.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
22

3-
constexpr int x = 1;
3+
constexpr int x = 1; // expected-note {{variable 'x' declared const here}}
44
constexpr int id(int x) { return x; }
55

66
void foo(void) {
7-
x = 2; // expected-error {{read-only variable is not assignable}}
7+
x = 2; // expected-error {{cannot assign to variable 'x' with const-qualified type 'const int'}}
88
int (*idp)(int) = id;
99
}
1010

0 commit comments

Comments
 (0)
Please sign in to comment.