[PATCH] D11334: [Sema] Call to deleted functions are supposed to be verboten

[Richard Smith]

On Tue, Jul 21, 2015 at 3:26 PM, Davide Italiano <dccitaliano at gmail.com>
wrote:

> On Tue, Jul 21, 2015 at 2:59 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> > rsmith added inline comments.
> >
> > ================
> > Comment at: lib/Sema/SemaOverload.cpp:11608
> > @@ +11607,3 @@
> > +
> > +    // Calls to deleted member functions are verboten.
> > +    if (Method && Method->isDeleted())
> > ----------------
> > This check should happen when we build the `MemberExpr`, not when we use
> it. It looks like the bug is in `Sema::BuildMemberReferenceExpr`, at around
> line 1050 of SemaExprMember.cpp:
> >
> >   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))
> >     return ExprError();
> >
> > This is wrong: we should `DiagnoseUseOfDecl` (including diagnosing the
> use of a deleted function) even for a virtual function. Which tests fail if
> we unconditionally `DiagnoseUseOfDecl` here?
> >
>
> Luckily only one.
>

Looks like this is testing a fix for
https://llvm.org/bugs/show_bug.cgi?id=4878 from way back in r81507 ... but
the fix is wrong, and it's not clear why PR4878 was even considered to be a
bug.

(As to why the fix is wrong, consider this:

struct S {
  virtual void f() __attribute__((deprecated));
  virtual void g() __attribute__((deprecated));
  virtual void g(int) __attribute__((deprecated));
};
void f(S s) { s.f(); s.g(); }

We warn on the call to g() but not the call to f().)

My preference is to update the below testcase to match the new behavior. If
that's actually a problem for someone, we can then get a description of
what the actual desired behavior is and implement that.


> FAIL: Clang :: SemaCXX/attr-deprecated.cpp (5733 of 8388)
> ******************** TEST 'Clang :: SemaCXX/attr-deprecated.cpp'
> FAILED ********************
> Script:
> --
> /exps/llvm2/build/./bin/clang -cc1 -internal-isystem
> /exps/llvm2/build/bin/../lib/clang/3.8.0/include -nostdsysteminc
> /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp -verify
> -fexceptions
> --
> Exit Code: 1
>
> Command Output (stderr):
> --
> error: 'warning' diagnostics seen but not expected:
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 34: 'f' is deprecated
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 50: 'f' is deprecated
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 65: 'f' is deprecated
> error: 'note' diagnostics seen but not expected:
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 29: 'f' has been explicitly marked deprecated here
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 29: 'f' has been explicitly marked deprecated here
>   File /exps/llvm2/tools/clang/test/SemaCXX/attr-deprecated.cpp Line
> 62: 'f' has been explicitly marked deprecated here
> 6 errors generated.
>
> --
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150721/2c2ea6b4/attachment.html>