On Tue, Jul 21, 2015 at 3:39 PM, Davide Italiano <dccitaliano@gmail.com> wrote:
On Tue, Jul 21, 2015 at 3:26 PM, Davide Italiano <dccitaliano@gmail.com> wrote:
> On Tue, Jul 21, 2015 at 2:59 PM, Richard Smith <richard@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.