[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 closed this revision. avt77 added a comment. Fixed by 324721. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
RKSimon added a comment. @avt77 Close this now that https://reviews.llvm.org/rL324721 has landed? https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 added a comment. Committed revision 324721. BTW, could you review https://reviews.llvm.org/D42728: it's rather similar to this one and rather small as well. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added a comment.
Thanks, that looks a lot better. One minor piece of feedback, but otherwise
LGTM.
Comment at: lib/Sema/SemaExpr.cpp:4402
+Qualifiers MemberQuals =
+Context.getCanonicalType(ResultType).getQualifiers();
+Qualifiers Combined = BaseQuals + MemberQuals;
You shouldn't have to get the canonical type here.
Comment at: test/Sema/assign.c:17
+ b[4] = 1; // expected-error {{cannot assign to variable 'b' with
const-qualified type 'const arr' (aka 'int const[10]')}}
+ b2[4] = 1;// expected-error {{cannot assign to variable 'b2' with
const-qualified type 'const int [10]'}}
}
These are nice improvements, thanks. There's still room for getting ever
better, but this is already nice progress.
https://reviews.llvm.org/D42530
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 updated this revision to Diff 133404.
avt77 added a comment.
I propagated qualifiers accordingly to rjmccall's suggestion. But I changed the
diagnostic: now it's more realistic from my point of view.
https://reviews.llvm.org/D42530
Files:
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprMember.cpp
test/Sema/assign.c
test/Sema/typedef-retain.c
test/SemaCXX/err_typecheck_assign_const.cpp
Index: test/SemaCXX/err_typecheck_assign_const.cpp
===
--- test/SemaCXX/err_typecheck_assign_const.cpp
+++ test/SemaCXX/err_typecheck_assign_const.cpp
@@ -129,3 +129,23 @@
Func ();
bar()() = 0; // expected-error {{read-only variable is not assignable}}
}
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+struct OhNo {
+ float4 v;
+ void AssignMe() const { v.x = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo::AssignMe' is declared const here}}
+};
+
+typedef float float4_2 __attribute__((__vector_size__(16)));
+struct OhNo2 {
+ float4_2 v;
+ void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo2::AssignMe' is declared const here}}
+};
+
+struct OhNo3 {
+ float v[4];
+ void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo3::AssignMe' is declared const here}}
+};
Index: test/Sema/typedef-retain.c
===
--- test/Sema/typedef-retain.c
+++ test/Sema/typedef-retain.c
@@ -16,8 +16,8 @@
typedef int a[5];
void test3() {
typedef const a b;
- b r;
- r[0]=10; // expected-error {{read-only variable is not assignable}}
+ b r; // expected-note {{variable 'r' declared const here}}
+ r[0] = 10; // expected-error {{cannot assign to variable 'r' with const-qualified type 'b' (aka 'int const[5]')}}
}
int test4(const a y) {
Index: test/Sema/assign.c
===
--- test/Sema/assign.c
+++ test/Sema/assign.c
@@ -11,10 +11,10 @@
typedef int arr[10];
void test3() {
- const arr b;
- const int b2[10];
- b[4] = 1; // expected-error {{read-only variable is not assignable}}
- b2[4] = 1; // expected-error {{read-only variable is not assignable}}
+ const arr b; // expected-note {{variable 'b' declared const here}}
+ const int b2[10]; // expected-note {{variable 'b2' declared const here}}
+ b[4] = 1; // expected-error {{cannot assign to variable 'b' with const-qualified type 'const arr' (aka 'int const[10]')}}
+ b2[4] = 1;// expected-error {{cannot assign to variable 'b2' with const-qualified type 'const int [10]'}}
}
typedef struct I {
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1623,10 +1623,14 @@
else
VK = BaseExpr.get()->getValueKind();
}
+
QualType ret = CheckExtVectorComponent(S, BaseType, VK, OpLoc,
Member, MemberLoc);
if (ret.isNull())
return ExprError();
+Qualifiers BaseQ =
+S.Context.getCanonicalType(BaseExpr.get()->getType()).getQualifiers();
+ret = S.Context.getQualifiedType(ret, BaseQ);
return new (S.Context)
ExtVectorElementExpr(ret, VK, BaseExpr.get(), *Member, MemberLoc);
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4395,8 +4395,14 @@
if (VK != VK_RValue)
OK = OK_VectorComponent;
-// FIXME: need to deal with const...
ResultType = VTy->getElementType();
+QualType BaseType = BaseExpr->getType();
+Qualifiers BaseQuals = BaseType.getQualifiers();
+Qualifiers MemberQuals =
+Context.getCanonicalType(ResultType).getQualifiers();
+Qualifiers Combined = BaseQuals + MemberQuals;
+if (Combined != MemberQuals)
+ ResultType = Context.getQualifiedType(ResultType, Combined);
} else if (LHSTy->isArrayType()) {
// If we see an array that wasn't promoted by
// DefaultFunctionArrayLvalueConversion, it must be an array that
@@ -10434,8 +10440,16 @@
// Static fields do not inherit constness from parents.
break;
}
- break;
-} // End MemberExpr
+ break; // End MemberExpr
+} else if (const ArraySubscriptExpr *ASE =
+ dyn_cast(E)) {
+ E = ASE->getBase()->IgnoreParenImpCasts();
+ continue;
+} else if (const ExtVectorElementExpr *EVE =
+
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added a comment. No, really, in CreateBuiltinArraySubscriptExpr and LookupMemberExpr, in the clauses where they handle vector types, you just need to propagate qualifiers from the base type like is done in BuildFieldReferenceExpr. There's even a FIXME about it in the former. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 updated this revision to Diff 132607.
avt77 added a comment.
I re-implemented the patch. Now it works properly with const methods in CXX
classes.
https://reviews.llvm.org/D42530
Files:
lib/AST/ExprClassification.cpp
lib/Sema/SemaExpr.cpp
test/Sema/assign.c
test/Sema/typedef-retain.c
test/SemaCXX/anonymous-union.cpp
test/SemaCXX/err_typecheck_assign_const.cpp
Index: test/SemaCXX/err_typecheck_assign_const.cpp
===
--- test/SemaCXX/err_typecheck_assign_const.cpp
+++ test/SemaCXX/err_typecheck_assign_const.cpp
@@ -129,3 +129,23 @@
Func ();
bar()() = 0; // expected-error {{read-only variable is not assignable}}
}
+
+typedef float float4 __attribute__((ext_vector_type(4)));
+struct OhNo {
+ float4 v;
+ void AssignMe() const { v.x = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo::AssignMe' is declared const here}}
+};
+
+typedef float float4_2 __attribute__((__vector_size__(16)));
+struct OhNo2 {
+ float4_2 v;
+ void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo2::AssignMe' is declared const here}}
+};
+
+struct OhNo3 {
+ float v[4];
+ void AssignMe() const { v[0] = 1; } // expected-error {{cannot assign to non-static data member within const member function 'AssignMe'}} \
+expected-note {{member function 'OhNo3::AssignMe' is declared const here}}
+};
Index: test/SemaCXX/anonymous-union.cpp
===
--- test/SemaCXX/anonymous-union.cpp
+++ test/SemaCXX/anonymous-union.cpp
@@ -40,7 +40,8 @@
}
void X::test_unqual_references_const() const { // expected-note 2{{member function 'X::test_unqual_references_const' is declared const here}}
- d = 0.0;
+ // TODO: it seems we should not see any error here becuase 'd' is mutable
+ d = 0.0; // expected-error {{read-only variable is not assignable}}
f2 = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
a = 0; // expected-error{{cannot assign to non-static data member within const member function 'test_unqual_references_const'}}
}
Index: test/Sema/typedef-retain.c
===
--- test/Sema/typedef-retain.c
+++ test/Sema/typedef-retain.c
@@ -16,8 +16,8 @@
typedef int a[5];
void test3() {
typedef const a b;
- b r;
- r[0]=10; // expected-error {{read-only variable is not assignable}}
+ b r; // expected-note {{variable 'r' declared const here}}
+ r[0]=10; // expected-error {{cannot assign to variable 'r' with const-qualified type 'b' (aka 'int const[5]')}}
}
int test4(const a y) {
Index: test/Sema/assign.c
===
--- test/Sema/assign.c
+++ test/Sema/assign.c
@@ -11,10 +11,10 @@
typedef int arr[10];
void test3() {
- const arr b;
- const int b2[10];
- b[4] = 1; // expected-error {{read-only variable is not assignable}}
- b2[4] = 1; // expected-error {{read-only variable is not assignable}}
+ const arr b; // expected-note {{variable 'b' declared const here}}
+ const int b2[10]; // expected-note {{variable 'b2' declared const here}}
+ b[4] = 1; // expected-error {{cannot assign to variable 'b' with const-qualified type 'const arr' (aka 'int const[10]')}}
+ b2[4] = 1; // expected-error {{cannot assign to variable 'b2' with const-qualified type 'const int [10]'}}
}
typedef struct I {
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10321,7 +10321,7 @@
/// 'const' due to being captured within a block?
enum NonConstCaptureKind { NCCK_None, NCCK_Block, NCCK_Lambda };
static NonConstCaptureKind isReferenceToNonConstCapture(Sema , Expr *E) {
- assert(E->isLValue() && E->getType().isConstQualified());
+ assert(E->isLValue());
E = E->IgnoreParens();
// Must be a reference to a declaration from an enclosing scope.
@@ -10401,10 +10401,8 @@
const ValueDecl *VD = ME->getMemberDecl();
if (const FieldDecl *Field = dyn_cast(VD)) {
// Mutable fields can be modified even if the class is const.
-if (Field->isMutable()) {
- assert(DiagnosticEmitted && "Expected diagnostic not emitted.");
+if (Field->isMutable())
break;
-}
if (!IsTypeModifiable(Field->getType(), IsDereference)) {
if (!DiagnosticEmitted) {
@@ -10434,8 +10432,16 @@
// Static fields do not inherit constness from parents.
break;
}
- break;
-} // End MemberExpr
+
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added a comment. If you just want a better diagnostic, there's quite a bit of machinery already set up to give more specific errors about why such-and-such l-value isn't modifiable. There may even be vector-specific diagnostics for that already, just triggered from the wrong place in the code. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 added a comment. In https://reviews.llvm.org/D42530#995227, @rjmccall wrote: > That's still just const-propagation. The const qualifier on the pointee type > of this should propagate to the l-value resulting from the member access, and > the vector-specific projection logic should continue to propagate const to > the type of the element l-value. I'm afraid it's wrong because from my point of view Clang generates wrong message for test OhNo3 (see err_typecheck_assign_const.cpp). It should not be "read-only variable is not assignable" because v[4] is not read-only variable. Any other non-const method is able to change this variable w/o any problems. The the same is for OhNo and OhNo2. BTW, gcc generates error: assignment of read-only location ‘*(const float*)(&((const OhNo2*)this)->OhNo2::v)’ and that's OK because we try to change the value through const-pointer-to-const. Am I right? https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added a comment. That's still just const-propagation. The const qualifier on the pointee type of this should propagate to the l-value resulting from the member access, and the vector-specific projection logic should continue to propagate const to the type of the element l-value. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 added a comment. In fact we have here another problem: it is not an attempt to assign to const variable but it is an attempt to use the field assingment inside const method: I'm working on it. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added inline comments. Comment at: lib/AST/ExprClassification.cpp:652 +if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified()) + return Cl::CM_ConstQualified; + avt77 wrote: > rjmccall wrote: > > Is there a reason why the places that compute the type of these l-value > > expressions don't propagate qualiifers? This hardly seems restricted to > > 'const'. > Do you mean ExtVectorElementExpr/ArraySubscriptExpr should retrun > isConstQualified() == true by themselves (and other qualifiers as well)? In the sense that Sema should create them with a type that is const-qualified when the underlying vector l-value is const-qualified, yes. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
avt77 added inline comments. Comment at: lib/AST/ExprClassification.cpp:652 +if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified()) + return Cl::CM_ConstQualified; + rjmccall wrote: > Is there a reason why the places that compute the type of these l-value > expressions don't propagate qualiifers? This hardly seems restricted to > 'const'. Do you mean ExtVectorElementExpr/ArraySubscriptExpr should retrun isConstQualified() == true by themselves (and other qualifiers as well)? https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42530: Clang permits assignment to vector/extvector elements in a const method
rjmccall added inline comments. Comment at: lib/AST/ExprClassification.cpp:652 +if (Ctx.getCanonicalType(ASE->getBase()->getType()).isConstQualified()) + return Cl::CM_ConstQualified; + Is there a reason why the places that compute the type of these l-value expressions don't propagate qualiifers? This hardly seems restricted to 'const'. https://reviews.llvm.org/D42530 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
