On Tue, Jul 21, 2015 at 3:26 PM, Davide Italiano <dccitali...@gmail.com> wrote:
> On Tue, Jul 21, 2015 at 2:59 PM, Richard Smith <rich...@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@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits