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

[Richard Smith]

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

> 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.
> >
> > 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.
> >
> > --
>
> Looking at the tests it seems we emit the warnings we previously
> emitted only for fully qualified functions also for non-fully
> qualified functions.
> If you think we should diagnose in both cases, I'll modify the test to
> take them in account.


Yes, our current behavior is inconsistent, different from GCC, and
surprising. Let's diagnose in both cases.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150721/772dd0d6/attachment.html>