[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: r242857 - [Sema] Diagnose use of declaration correctly.
From:       Richard Smith <richard () metafoo ! co ! uk>
Date:       2015-07-22 0:37:45
Message-ID: CAOfiQqkwSkW3SaHdhBiBy3VD0QiJkehifW6B_ZdK_t_wUSgq2w () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


On Tue, Jul 21, 2015 at 5:30 PM, Davide Italiano <davide@freebsd.org> wrote:

> Author: davide
> Date: Tue Jul 21 19:30:58 2015
> New Revision: 242857
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=242857&view=rev
> Log:
> [Sema] Diagnose use of declaration correctly.
> 
> Before we skipped that for virtual functions not fully qualified (r81507).
> This commit basically reverts this to the older behaviour, which seems
> more consistent. We now also correctly consider ill-formed calls to deleted
> member functions, which were silently passed before in some cases.
> The review contains the whole discussion.
> 
> PR:             20268
> Differential Revision:   http://reviews.llvm.org/D11334
> 
> Added:
> cfe/trunk/test/SemaCXX/deleted-function-access.cpp
> Modified:
> cfe/trunk/lib/Sema/SemaExprMember.cpp
> cfe/trunk/test/SemaCXX/attr-deprecated.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=242857&r1=242856&r2=242857&view=diff
>  
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Jul 21 19:30:58 2015
> @@ -1042,16 +1042,8 @@ Sema::BuildMemberReferenceExpr(Expr *Bas
> BaseExpr = new (Context) CXXThisExpr(Loc,
> BaseExprType,/*isImplicit=*/true);
> }
> 
> -  bool ShouldCheckUse = true;
> -  if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(MemberDecl)) {
> -    // Don't diagnose the use of a virtual member function unless it's
> -    // explicitly qualified.
> -    if (MD->isVirtual() && !SS.isSet())
> -      ShouldCheckUse = false;
> -  }
> -
> // Check the use of this member.
> -  if (ShouldCheckUse && DiagnoseUseOfDecl(MemberDecl, MemberLoc))
> +  if (DiagnoseUseOfDecl(MemberDecl, MemberLoc))
> return ExprError();
> 
> if (FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl))
> 
> Modified: cfe/trunk/test/SemaCXX/attr-deprecated.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/attr-deprecated.cpp?rev=242857&r1=242856&r2=242857&view=diff
>  
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/attr-deprecated.cpp (original)
> +++ cfe/trunk/test/SemaCXX/attr-deprecated.cpp Tue Jul 21 19:30:58 2015
> @@ -26,12 +26,12 @@ void A::h(A* a)
> }
> 
> struct B {
> -  virtual void f() __attribute__((deprecated)); // expected-note 4 {{'f'
> has been explicitly marked deprecated here}}
> +  virtual void f() __attribute__((deprecated)); // expected-note 6 {{'f'
> has been explicitly marked deprecated here}}
> void g();
> };
> 
> void B::g() {
> -  f();
> +  f(); // expected-warning{{'f' is deprecated}}
> B::f(); // expected-warning{{'f' is deprecated}}
> }
> 
> @@ -47,7 +47,7 @@ void C::g() {
> }
> 
> void f(B* b, C *c) {
> -  b->f();
> +  b->f(); // expected-warning{{'f' is deprecated}}
> b->B::f(); // expected-warning{{'f' is deprecated}}
> 
> c->f();
> @@ -59,10 +59,10 @@ struct D {
> virtual void f() __attribute__((deprecated));
> };
> 
> -void D::f() { }
> +void D::f() { } // expected-note{{'f' has been explicitly marked
> deprecated here}}
> 
> void f(D* d) {
> -  d->f();
> +  d->f(); // expected-warning{{'f' is deprecated}}
> }
> 

Can you also add some tests here for the case where the deprecated virtual
function is overloaded?

Added: cfe/trunk/test/SemaCXX/deleted-function-access.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/deleted-function-access.cpp?rev=242857&view=auto
>  
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/deleted-function-access.cpp (added)
> +++ cfe/trunk/test/SemaCXX/deleted-function-access.cpp Tue Jul 21 19:30:58
> 2015
> @@ -0,0 +1,6 @@
> +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
> +
> +struct S {
> +    virtual void f() = delete; //expected-note{{'f' has been explicitly
> marked deleted here}}
> +    void g() { f(); } //expected-error{{attempt to use a deleted
> function}}
> +};
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 


[Attachment #5 (text/html)]

<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jul 21, 2015 \
at 5:30 PM, Davide Italiano <span dir="ltr">&lt;<a href="mailto:davide@freebsd.org" \
target="_blank">davide@freebsd.org</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
                solid;padding-left:1ex">Author: davide<br>
Date: Tue Jul 21 19:30:58 2015<br>
New Revision: 242857<br>
<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject-3Frev-3D242857-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW \
_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=cMgb8JSF1cFkvLhO0w1KIqHAhUfJJ9qOPxIaTD0ULkU&s=0uJR0P1wbcB0dYec2C7AKV9fEYpbC1g20hRct1mI8r8&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=242857&amp;view=rev</a><br>
 Log:<br>
[Sema] Diagnose use of declaration correctly.<br>
<br>
Before we skipped that for virtual functions not fully qualified (r81507).<br>
This commit basically reverts this to the older behaviour, which seems<br>
more consistent. We now also correctly consider ill-formed calls to deleted<br>
member functions, which were silently passed before in some cases.<br>
The review contains the whole discussion.<br>
<br>
PR:                    20268<br>
Differential Revision:     <a \
href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D11334&d=Aw \
MFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=cMgb8JSF \
1cFkvLhO0w1KIqHAhUfJJ9qOPxIaTD0ULkU&s=31DtR-6wxUIa_ZB1EyFHS3yR8XBUrOq_Wj4G8WOSis4&e=" \
rel="noreferrer" target="_blank">http://reviews.llvm.org/D11334</a><br> <br>
Added:<br>
      cfe/trunk/test/SemaCXX/deleted-function-access.cpp<br>
Modified:<br>
      cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
      cfe/trunk/test/SemaCXX/attr-deprecated.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_lib_Sema_SemaExprMember.cpp-3Frev-3D242857-26r1-3D242856-26r2-3D2 \
42857-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_A \
SROnREeq0cCxk&m=cMgb8JSF1cFkvLhO0w1KIqHAhUfJJ9qOPxIaTD0ULkU&s=g-5xejvbrf-hyPAKpIX9VnObbdHT32_weLIGoLGGPCU&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sem \
a/SemaExprMember.cpp?rev=242857&amp;r1=242856&amp;r2=242857&amp;view=diff</a><br> \
                ==============================================================================<br>
                
--- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprMember.cpp Tue Jul 21 19:30:58 2015<br>
@@ -1042,16 +1042,8 @@ Sema::BuildMemberReferenceExpr(Expr *Bas<br>
        BaseExpr = new (Context) CXXThisExpr(Loc, \
BaseExprType,/*isImplicit=*/true);<br>  }<br>
<br>
-   bool ShouldCheckUse = true;<br>
-   if (CXXMethodDecl *MD = dyn_cast&lt;CXXMethodDecl&gt;(MemberDecl)) {<br>
-      // Don&#39;t diagnose the use of a virtual member function unless it&#39;s<br>
-      // explicitly qualified.<br>
-      if (MD-&gt;isVirtual() &amp;&amp; !SS.isSet())<br>
-         ShouldCheckUse = false;<br>
-   }<br>
-<br>
     // Check the use of this member.<br>
-   if (ShouldCheckUse &amp;&amp; DiagnoseUseOfDecl(MemberDecl, MemberLoc))<br>
+   if (DiagnoseUseOfDecl(MemberDecl, MemberLoc))<br>
        return ExprError();<br>
<br>
     if (FieldDecl *FD = dyn_cast&lt;FieldDecl&gt;(MemberDecl))<br>
<br>
Modified: cfe/trunk/test/SemaCXX/attr-deprecated.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_test_SemaCXX_attr-2Ddeprecated.cpp-3Frev-3D242857-26r1-3D242856-2 \
6r2-3D242857-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70K \
dZISM_ASROnREeq0cCxk&m=cMgb8JSF1cFkvLhO0w1KIqHAhUfJJ9qOPxIaTD0ULkU&s=vWJu_ABa_Uhdy9Iq1QGre0Ayzh2bhMLDTLAoPTLyN1s&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Se \
maCXX/attr-deprecated.cpp?rev=242857&amp;r1=242856&amp;r2=242857&amp;view=diff</a><br>
 ==============================================================================<br>
--- cfe/trunk/test/SemaCXX/attr-deprecated.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/attr-deprecated.cpp Tue Jul 21 19:30:58 2015<br>
@@ -26,12 +26,12 @@ void A::h(A* a)<br>
  }<br>
<br>
  struct B {<br>
-   virtual void f() __attribute__((deprecated)); // expected-note 4 {{&#39;f&#39; \
has been explicitly marked deprecated here}}<br> +   virtual void f() \
__attribute__((deprecated)); // expected-note 6 {{&#39;f&#39; has been explicitly \
marked deprecated here}}<br>  void g();<br>
  };<br>
<br>
  void B::g() {<br>
-   f();<br>
+   f(); // expected-warning{{&#39;f&#39; is deprecated}}<br>
     B::f(); // expected-warning{{&#39;f&#39; is deprecated}}<br>
  }<br>
<br>
@@ -47,7 +47,7 @@ void C::g() {<br>
  }<br>
<br>
  void f(B* b, C *c) {<br>
-   b-&gt;f();<br>
+   b-&gt;f(); // expected-warning{{&#39;f&#39; is deprecated}}<br>
     b-&gt;B::f(); // expected-warning{{&#39;f&#39; is deprecated}}<br>
<br>
     c-&gt;f();<br>
@@ -59,10 +59,10 @@ struct D {<br>
     virtual void f() __attribute__((deprecated));<br>
  };<br>
<br>
-void D::f() { }<br>
+void D::f() { } // expected-note{{&#39;f&#39; has been explicitly marked deprecated \
here}}<br> <br>
  void f(D* d) {<br>
-   d-&gt;f();<br>
+   d-&gt;f(); // expected-warning{{&#39;f&#39; is deprecated}}<br>
  }<br></blockquote><div><br></div><div>Can you also add some tests here for the case \
where the deprecated virtual function is overloaded?</div><div><br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
                solid;padding-left:1ex">
Added: cfe/trunk/test/SemaCXX/deleted-function-access.cpp<br>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm \
-2Dproject_cfe_trunk_test_SemaCXX_deleted-2Dfunction-2Daccess.cpp-3Frev-3D242857-26vie \
w-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cC \
xk&m=cMgb8JSF1cFkvLhO0w1KIqHAhUfJJ9qOPxIaTD0ULkU&s=EWtxB5wi0ZMffizBMZTCkDmbKEAcqYXlNEnDHyu6QPo&e=" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/deleted-function-access.cpp?rev=242857&amp;view=auto</a><br>
 ==============================================================================<br>
--- cfe/trunk/test/SemaCXX/deleted-function-access.cpp (added)<br>
+++ cfe/trunk/test/SemaCXX/deleted-function-access.cpp Tue Jul 21 19:30:58 2015<br>
@@ -0,0 +1,6 @@<br>
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s<br>
+<br>
+struct S {<br>
+      virtual void f() = delete; //expected-note{{&#39;f&#39; has been explicitly \
marked deleted here}}<br> +      void g() { f(); } //expected-error{{attempt to use a \
deleted function}}<br> +};<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" \
target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br> \
</blockquote></div><br></div></div>



_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic