r313628 - [Sema] Disallow assigning record lvalues with nested const-qualified fields.
Bjorn Pettersson via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 19 06:10:30 PDT 2017
Author: bjope
Date: Tue Sep 19 06:10:30 2017
New Revision: 313628
URL: http://llvm.org/viewvc/llvm-project?rev=313628&view=rev
Log:
[Sema] Disallow assigning record lvalues with nested const-qualified fields.
Summary:
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 .
Committing on behalf of bevinh (Bevin Hansson).
Reviewers: rtrieu, rsmith, bjope
Reviewed By: bjope
Subscribers: Ka-Ka, rogfer01, bjope, fhahn, cfe-commits
Differential Revision: https://reviews.llvm.org/D37254
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprClassification.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/assign.c
Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Sep 19 06:10:30 2017
@@ -275,6 +275,7 @@ public:
MLV_LValueCast, // Specialized form of MLV_InvalidExpression.
MLV_IncompleteType,
MLV_ConstQualified,
+ MLV_ConstQualifiedField,
MLV_ConstAddrSpace,
MLV_ArrayType,
MLV_NoSetterProperty,
@@ -324,6 +325,7 @@ public:
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
Modified: cfe/trunk/include/clang/AST/Type.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Tue Sep 19 06:10:30 2017
@@ -3791,10 +3791,9 @@ public:
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); }
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Sep 19 06:10:30 2017
@@ -5888,6 +5888,8 @@ def err_typecheck_assign_const : Error<
"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<
@@ -5895,7 +5897,8 @@ def note_typecheck_assign_const : Note<
"function %1 which returns const-qualified type %2 declared here|"
"variable %1 declared const here|"
"%select{non-|}1static data member %2 declared const here|"
- "member function %q1 is declared const here}0">;
+ "member function %q1 is declared const here|"
+ "%select{|nested }1data member %2 declared const here}0">;
def warn_mixed_sign_comparison : Warning<
"comparison of integers of different signs: %0 and %1">,
Modified: cfe/trunk/lib/AST/ExprClassification.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprClassification.cpp?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ExprClassification.cpp (original)
+++ cfe/trunk/lib/AST/ExprClassification.cpp Tue Sep 19 06:10:30 2017
@@ -641,7 +641,7 @@ static Cl::ModifiableType IsModifiable(A
// 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 @@ Expr::isModifiableLvalue(ASTContext &Ctx
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;
Modified: cfe/trunk/lib/AST/Type.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Sep 19 06:10:30 2017
@@ -2994,6 +2994,19 @@ bool TagType::isBeingDefined() const {
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
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Sep 19 06:10:30 2017
@@ -10260,22 +10260,23 @@ static bool IsTypeModifiable(QualType Ty
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
@@ -10385,6 +10386,66 @@ static void DiagnoseConstAssignment(Sema
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)
+ << NestedConstMember << IsNested << 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) {
@@ -10460,6 +10521,9 @@ static bool CheckForModifiableLvalue(Exp
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;
Modified: cfe/trunk/test/Sema/assign.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/assign.c?rev=313628&r1=313627&r2=313628&view=diff
==============================================================================
--- cfe/trunk/test/Sema/assign.c (original)
+++ cfe/trunk/test/Sema/assign.c Tue Sep 19 06:10:30 2017
@@ -16,3 +16,46 @@ void test3() {
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 4{{nested data member 'a' declared const here}} \
+ expected-note 6{{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'}}
+}
More information about the cfe-commits
mailing list