[PATCH] D11940: don't diagnose -Wunused-parameter in virtual method or method that overrides base class method

[Nico Weber via cfe-commits]

On Tue, Aug 11, 2015 at 11:32 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Aug 11, 2015 at 8:46 AM, Nico Weber via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Can't you just change your signature to
>>
>>   virtual void a(int /* x */) {}
>>
>> in these cases?
>>
>
> You could - does it add much value to do that, though?
>

If you enable this warning, you probably want to know about unused
parameters, independent of if your function is virtual or not, no?


> (perhaps it does - it means you express the intent that the parameter is
> not used and the compiler helps you check that (so that for the parameters
> you think /should/ be used (you haven't commented out their name but
> accidentally shadow or otherwise fail to reference, you still get a
> warning))
>
> - David
>
>
>>
>> On Tue, Aug 11, 2015 at 6:43 AM, Daniel Marjamäki <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> danielmarjamaki created this revision.
>>> danielmarjamaki added a reviewer: krememek.
>>> danielmarjamaki added a subscriber: cfe-commits.
>>>
>>> Don't diagnose -Wunused-parameter in methods that override other methods
>>> because the overridden methods might use the parameter
>>>
>>> Don't diagnose -Wunused-parameter in virtual methods because these might
>>> be overriden by other methods that use the parameter.
>>>
>>> Such diagnostics could be more accurately written if they are based on
>>> whole-program-analysis that establish if such parameter is unused in all
>>> methods.
>>>
>>>
>>>
>>> http://reviews.llvm.org/D11940
>>>
>>> Files:
>>>   lib/Sema/SemaDecl.cpp
>>>   test/SemaCXX/warn-unused-parameters.cpp
>>>
>>> Index: test/SemaCXX/warn-unused-parameters.cpp
>>> ===================================================================
>>> --- test/SemaCXX/warn-unused-parameters.cpp
>>> +++ test/SemaCXX/warn-unused-parameters.cpp
>>> @@ -32,3 +32,20 @@
>>>    auto l = [&t...]() { return sizeof...(s); };
>>>    return l();
>>>  }
>>> +
>>> +// Don't diagnose virtual methods or methods that override base class
>>> +// methods.
>>> +class Base {
>>> +public:
>>> +  virtual void f(int x);
>>> +};
>>> +
>>> +class Derived : public Base {
>>> +public:
>>> +  // Don't warn in overridden methods.
>>> +  virtual void f(int x) {}
>>> +
>>> +  // Don't warn in virtual methods.
>>> +  virtual void a(int x) {}
>>> +};
>>> +
>>> Index: lib/Sema/SemaDecl.cpp
>>> ===================================================================
>>> --- lib/Sema/SemaDecl.cpp
>>> +++ lib/Sema/SemaDecl.cpp
>>> @@ -10797,8 +10797,13 @@
>>>
>>>      if (!FD->isInvalidDecl()) {
>>>        // Don't diagnose unused parameters of defaulted or deleted
>>> functions.
>>> -      if (!FD->isDeleted() && !FD->isDefaulted())
>>> -        DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>>> +      if (!FD->isDeleted() && !FD->isDefaulted()) {
>>> +        // Don't diagnose unused parameters in virtual methods or
>>> +        // in methods that override base class methods.
>>> +        const auto MD = dyn_cast<CXXMethodDecl>(FD);
>>> +        if (!MD || (MD->size_overridden_methods() == 0U &&
>>> !MD->isVirtual()))
>>> +          DiagnoseUnusedParameters(FD->param_begin(), FD->param_end());
>>> +      }
>>>        DiagnoseSizeOfParametersAndReturnValue(FD->param_begin(),
>>> FD->param_end(),
>>>                                               FD->getReturnType(), FD);
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150811/71426244/attachment.html>