[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
This revision was automatically updated to reflect the committed changes. Closed by commit rL305126: 27037: Use correct CVR qualifier on an upcast on method pointer call (authored by rsmith). Changed prior to commit: https://reviews.llvm.org/D33875?vs=101536&id=102081#toc Repository: rL LLVM https://reviews.llvm.org/D33875 Files: cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/test/SemaCXX/PR27037.cpp Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp === --- cfe/trunk/lib/Sema/SemaExprCXX.cpp +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp @@ -5106,7 +5106,9 @@ return QualType(); // Cast LHS to type of use. -QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +QualType UseType = Context.getQualifiedType(Class, LHSType.getQualifiers()); +if (isIndirect) + UseType = Context.getPointerType(UseType); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK, &BasePath); Index: cfe/trunk/test/SemaCXX/PR27037.cpp === --- cfe/trunk/test/SemaCXX/PR27037.cpp +++ cfe/trunk/test/SemaCXX/PR27037.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct A { + void f(); +}; + +struct B : A {}; + +void m() { + const B b; + (b.*&B::f)(); // expected-error{{drops 'const' qualifier}} + ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} +} Index: cfe/trunk/lib/Sema/SemaExprCXX.cpp === --- cfe/trunk/lib/Sema/SemaExprCXX.cpp +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp @@ -5106,7 +5106,9 @@ return QualType(); // Cast LHS to type of use. -QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +QualType UseType = Context.getQualifiedType(Class, LHSType.getQualifiers()); +if (isIndirect) + UseType = Context.getPointerType(UseType); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK, &BasePath); Index: cfe/trunk/test/SemaCXX/PR27037.cpp === --- cfe/trunk/test/SemaCXX/PR27037.cpp +++ cfe/trunk/test/SemaCXX/PR27037.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct A { + void f(); +}; + +struct B : A {}; + +void m() { + const B b; + (b.*&B::f)(); // expected-error{{drops 'const' qualifier}} + ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
tzik marked an inline comment as done. tzik added a comment. In https://reviews.llvm.org/D33875#774293, @rsmith wrote: > Looks good to me, thanks! Do you need someone to commit this for you? Yes, could you commit this? https://reviews.llvm.org/D33875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Looks good to me, thanks! Do you need someone to commit this for you? https://reviews.llvm.org/D33875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
tzik marked an inline comment as done. tzik added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:5108 QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +UseType = UseType.withCVRQualifiers(LHS.get()->getType().getCVRQualifiers()); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); rsmith wrote: > In the "indirect" case, the cv-qualifiers should be taken from the pointee > type of the LHS and applied to the pointee type of UseType. I believe this > patch will not be enough to cause us to reject the indirect version of your > testcase: > > ``` > ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} > ``` > > Moreover, we should be preserving all qualifiers, not just cvr-qualifiers; > for example, this should also preserve the address space. Make sense. OK, I updated the CL to cover that cases. Could you take another look? https://reviews.llvm.org/D33875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
tzik updated this revision to Diff 101536. tzik added a comment. Cover indirect case and non-CVR qualifiers https://reviews.llvm.org/D33875 Files: lib/Sema/SemaExprCXX.cpp test/SemaCXX/PR27037.cpp Index: test/SemaCXX/PR27037.cpp === --- /dev/null +++ test/SemaCXX/PR27037.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct A { + void f(); +}; + +struct B : A {}; + +void m() { + const B b; + (b.*&B::f)(); // expected-error{{drops 'const' qualifier}} + ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} +} Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -5104,7 +5104,9 @@ return QualType(); // Cast LHS to type of use. -QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +QualType UseType = Context.getQualifiedType(Class, LHSType.getQualifiers()); +if (isIndirect) + UseType = Context.getPointerType(UseType); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK, &BasePath); Index: test/SemaCXX/PR27037.cpp === --- /dev/null +++ test/SemaCXX/PR27037.cpp @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +struct A { + void f(); +}; + +struct B : A {}; + +void m() { + const B b; + (b.*&B::f)(); // expected-error{{drops 'const' qualifier}} + ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} +} Index: lib/Sema/SemaExprCXX.cpp === --- lib/Sema/SemaExprCXX.cpp +++ lib/Sema/SemaExprCXX.cpp @@ -5104,7 +5104,9 @@ return QualType(); // Cast LHS to type of use. -QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +QualType UseType = Context.getQualifiedType(Class, LHSType.getQualifiers()); +if (isIndirect) + UseType = Context.getPointerType(UseType); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); LHS = ImpCastExprToType(LHS.get(), UseType, CK_DerivedToBase, VK, &BasePath); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
rsmith added inline comments. Comment at: lib/Sema/SemaExprCXX.cpp:5108 QualType UseType = isIndirect ? Context.getPointerType(Class) : Class; +UseType = UseType.withCVRQualifiers(LHS.get()->getType().getCVRQualifiers()); ExprValueKind VK = isIndirect ? VK_RValue : LHS.get()->getValueKind(); In the "indirect" case, the cv-qualifiers should be taken from the pointee type of the LHS and applied to the pointee type of UseType. I believe this patch will not be enough to cause us to reject the indirect version of your testcase: ``` ((&b)->*&B::f)(); // expected-error{{drops 'const' qualifier}} ``` Moreover, we should be preserving all qualifiers, not just cvr-qualifiers; for example, this should also preserve the address space. https://reviews.llvm.org/D33875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33875: PR27037: Use correct CVR qualifier on an upcast on method pointer call
tzik added a comment. Hi, Richard. Could you PTAL to this? https://reviews.llvm.org/D33875 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits