bevinh created this revision.
According to C99 6.3.2.1p1, structs and unions with nested
const-qualified fields (that is, const-qualified fields
declared at some recursive level of the aggregate) are not
modifiable lvalues. However, Clang permits assignments of
these lvalues.
With this patch, we both prohibit the assignment of records
with const-qualified fields and emit a best-effort diagnostic.
This fixes https://bugs.llvm.org/show_bug.cgi?id=31796 .
https://reviews.llvm.org/D37254
Files:
include/clang/AST/Expr.h
include/clang/AST/Type.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/AST/ExprClassification.cpp
lib/AST/Type.cpp
lib/Sema/SemaExpr.cpp
test/Sema/assign.c
Index: test/Sema/assign.c
===================================================================
--- test/Sema/assign.c
+++ test/Sema/assign.c
@@ -16,3 +16,45 @@
b[4] = 1; // expected-error {{read-only variable is not assignable}}
b2[4] = 1; // expected-error {{read-only variable is not assignable}}
}
+
+typedef struct I {
+ const int a; // expected-note 10{{non-static data member 'a' declared const here}}
+} I;
+typedef struct J {
+ struct I i;
+} J;
+typedef struct K {
+ struct J *j;
+} K;
+
+void testI(struct I i1, struct I i2) {
+ i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}}
+}
+void testJ1(struct J j1, struct J j2) {
+ j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}}
+}
+void testJ2(struct J j, struct I i) {
+ j.i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
+void testK1(struct K k, struct J j) {
+ *(k.j) = j; // expected-error {{cannot assign to lvalue with nested const-qualified data member 'a'}}
+}
+void testK2(struct K k, struct I i) {
+ k.j->i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
+
+void testI_(I i1, I i2) {
+ i1 = i2; // expected-error {{cannot assign to variable 'i1' with const-qualified data member 'a'}}
+}
+void testJ1_(J j1, J j2) {
+ j1 = j2; // expected-error {{cannot assign to variable 'j1' with nested const-qualified data member 'a'}}
+}
+void testJ2_(J j, I i) {
+ j.i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
+void testK1_(K k, J j) {
+ *(k.j) = j; // expected-error {{cannot assign to lvalue with nested const-qualified data member 'a'}}
+}
+void testK2_(K k, I i) {
+ k.j->i = i; // expected-error {{cannot assign to non-static data member 'i' with const-qualified data member 'a'}}
+}
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10176,22 +10176,23 @@
return !Ty.isConstQualified();
}
+// Update err_typecheck_assign_const and note_typecheck_assign_const
+// when this enum is changed.
+enum {
+ ConstFunction,
+ ConstVariable,
+ ConstMember,
+ ConstMethod,
+ NestedConstMember,
+ ConstUnknown, // Keep as last element
+};
+
/// Emit the "read-only variable not assignable" error and print notes to give
/// more information about why the variable is not assignable, such as pointing
/// to the declaration of a const variable, showing that a method is const, or
/// that the function is returning a const reference.
static void DiagnoseConstAssignment(Sema &S, const Expr *E,
SourceLocation Loc) {
- // Update err_typecheck_assign_const and note_typecheck_assign_const
- // when this enum is changed.
- enum {
- ConstFunction,
- ConstVariable,
- ConstMember,
- ConstMethod,
- ConstUnknown, // Keep as last element
- };
-
SourceRange ExprRange = E->getSourceRange();
// Only emit one error on the first const found. All other consts will emit
@@ -10301,6 +10302,66 @@
S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange << ConstUnknown;
}
+enum OriginalExprKind {
+ OEK_Variable,
+ OEK_Member,
+ OEK_LValue
+};
+
+static void DiagnoseRecursiveConstFields(Sema &S, const ValueDecl *VD,
+ const RecordType *Ty,
+ SourceLocation Loc, SourceRange Range,
+ OriginalExprKind OEK,
+ bool &DiagnosticEmitted,
+ bool IsNested = false) {
+ // We walk the record hierarchy breadth-first to ensure that we print
+ // diagnostics in field nesting order.
+ // First, check every field for constness.
+ for (const FieldDecl *Field : Ty->getDecl()->fields()) {
+ if (Field->getType().isConstQualified()) {
+ if (!DiagnosticEmitted) {
+ S.Diag(Loc, diag::err_typecheck_assign_const)
+ << Range << NestedConstMember << OEK << VD
+ << IsNested << Field;
+ DiagnosticEmitted = true;
+ }
+ S.Diag(Field->getLocation(), diag::note_typecheck_assign_const)
+ << ConstMember << false /*non-static*/ << Field
+ << Field->getType() << Field->getSourceRange();
+ }
+ }
+ // Then, recurse.
+ for (const FieldDecl *Field : Ty->getDecl()->fields()) {
+ QualType FTy = Field->getType();
+ if (const RecordType *FieldRecTy = FTy->getAs<RecordType>())
+ DiagnoseRecursiveConstFields(S, VD, FieldRecTy, Loc, Range,
+ OEK, DiagnosticEmitted, true);
+ }
+}
+
+/// Emit an error for the case where a record we are trying to assign to has a
+/// const-qualified field somewhere in its hierarchy.
+static void DiagnoseRecursiveConstFields(Sema &S, const Expr *E,
+ SourceLocation Loc) {
+ QualType Ty = E->getType();
+ assert(Ty->isRecordType() && "lvalue was not record?");
+ SourceRange Range = E->getSourceRange();
+ const RecordType *RTy = Ty.getCanonicalType()->getAs<RecordType>();
+ bool DiagEmitted = false;
+
+ if (const MemberExpr *ME = dyn_cast<MemberExpr>(E))
+ DiagnoseRecursiveConstFields(S, ME->getMemberDecl(), RTy, Loc,
+ Range, OEK_Member, DiagEmitted);
+ else if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
+ DiagnoseRecursiveConstFields(S, DRE->getDecl(), RTy, Loc,
+ Range, OEK_Variable, DiagEmitted);
+ else
+ DiagnoseRecursiveConstFields(S, nullptr, RTy, Loc,
+ Range, OEK_LValue, DiagEmitted);
+ if (!DiagEmitted)
+ DiagnoseConstAssignment(S, E, Loc);
+}
+
/// CheckForModifiableLvalue - Verify that E is a modifiable lvalue. If not,
/// emit an error and return true. If so, return false.
static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
@@ -10376,6 +10437,9 @@
case Expr::MLV_ConstAddrSpace:
DiagnoseConstAssignment(S, E, Loc);
return true;
+ case Expr::MLV_ConstQualifiedField:
+ DiagnoseRecursiveConstFields(S, E, Loc);
+ return true;
case Expr::MLV_ArrayType:
case Expr::MLV_ArrayTemporary:
DiagID = diag::err_typecheck_array_not_modifiable_lvalue;
Index: lib/AST/Type.cpp
===================================================================
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2992,6 +2992,19 @@
return getDecl()->isBeingDefined();
}
+bool RecordType::hasConstFields() const {
+ for (FieldDecl *FD : getDecl()->fields()) {
+ QualType FieldTy = FD->getType();
+ if (FieldTy.isConstQualified())
+ return true;
+ FieldTy = FieldTy.getCanonicalType();
+ if (const RecordType *FieldRecTy = FieldTy->getAs<RecordType>())
+ if (FieldRecTy->hasConstFields())
+ return true;
+ }
+ return false;
+}
+
bool AttributedType::isQualifier() const {
switch (getAttrKind()) {
// These are type qualifiers in the traditional C sense: they annotate
Index: lib/AST/ExprClassification.cpp
===================================================================
--- lib/AST/ExprClassification.cpp
+++ lib/AST/ExprClassification.cpp
@@ -641,7 +641,7 @@
// Records with any const fields (recursively) are not modifiable.
if (const RecordType *R = CT->getAs<RecordType>())
if (R->hasConstFields())
- return Cl::CM_ConstQualified;
+ return Cl::CM_ConstQualifiedField;
return Cl::CM_Modifiable;
}
@@ -695,6 +695,7 @@
llvm_unreachable("CM_LValueCast and CL_LValue don't match");
case Cl::CM_NoSetterProperty: return MLV_NoSetterProperty;
case Cl::CM_ConstQualified: return MLV_ConstQualified;
+ case Cl::CM_ConstQualifiedField: return MLV_ConstQualifiedField;
case Cl::CM_ConstAddrSpace: return MLV_ConstAddrSpace;
case Cl::CM_ArrayType: return MLV_ArrayType;
case Cl::CM_IncompleteType: return MLV_IncompleteType;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5881,6 +5881,8 @@
"cannot assign to %select{non-|}1static data member %2 "
"with const-qualified type %3|"
"cannot assign to non-static data member within const member function %1|"
+ "cannot assign to %select{variable %2|non-static data member %2|lvalue}1 "
+ "with %select{|nested }3const-qualified data member %4|"
"read-only variable is not assignable}0">;
def note_typecheck_assign_const : Note<
Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -3791,10 +3791,9 @@
return reinterpret_cast<RecordDecl*>(TagType::getDecl());
}
- // FIXME: This predicate is a helper to QualType/Type. It needs to
- // recursively check all fields for const-ness. If any field is declared
- // const, it needs to return false.
- bool hasConstFields() const { return false; }
+ /// Recursively check all fields in the record for const-ness. If any field
+ /// is declared const, return true. Otherwise, return false.
+ bool hasConstFields() const;
bool isSugared() const { return false; }
QualType desugar() const { return QualType(this, 0); }
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -275,6 +275,7 @@
MLV_LValueCast, // Specialized form of MLV_InvalidExpression.
MLV_IncompleteType,
MLV_ConstQualified,
+ MLV_ConstQualifiedField,
MLV_ConstAddrSpace,
MLV_ArrayType,
MLV_NoSetterProperty,
@@ -324,6 +325,7 @@
CM_LValueCast, // Same as CM_RValue, but indicates GCC cast-as-lvalue ext
CM_NoSetterProperty,// Implicit assignment to ObjC property without setter
CM_ConstQualified,
+ CM_ConstQualifiedField,
CM_ConstAddrSpace,
CM_ArrayType,
CM_IncompleteType
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
