[llvm] r266183 - Don't use misc-unused-parameters check on LLVM.

[David Blaikie via llvm-commits]

On Wed, Apr 13, 2016 at 6:10 PM, Alexander Kornienko <alexfh at google.com>
wrote:

> There was no discussion, but I don't anticipate any objections, because
> this check neither has any support in
> http://llvm.org/docs/CodingStandards.html nor corresponds to a
> wide-spread practice in the LLVM code.
>
It's the sort of thing I wouldn't really expect to be formalized and that
I'd expect the codebase to rot /very/ quickly in the absence of any
warning/check, even if it was formalized. So neither of these things seem
surprising to me, but I wouldn't necessarily conclude that the check isn't
applicable to LLVM because of them.

> There are a handful of methods using this convention and hundreds that
> don't.
>
"convention" makes it sound deliberate. I imagine most of the dead
parameters are just a function of refactoring (or API constraints - I
imagine that the check can handle the case of an unused parameter in a
virtual override, but may not catch unused parameters where a matching
signature is needed for other reasons (calling through a function pointer,
etc)? I imagine we might have a handful of those in the codebase, but would
be surprised if there are a lot)

> This is the single most noisy check on LLVM code that was turned on before
> this commit. If we want to use this check, we first need to adopt the
> corresponding rule and clean up thousands of warnings from this check.
>
Isn't that (one of) the point(s) of clang-tidy, though - to allow us to
hold a higher bar for new code, without having to cleanup the codebase to
make it check-clean?

(side note: I actually haven't seen/heard/understood how clang-tidy is
being integrated into any LLVM developer workflow - is it? It'd be great to
have a few different levels of integration - in the build for the things
that just aren't clang diagnostics because they're project specific (not
because they have a low false positive or because we don't want to cleanup
LLVM first) - actually it'd be great if clang itself had some way to
integrate /thoes/ ones. Then another level for the "going forward" kind -
just restrict itself to things in my local change so I don't introduce more
mistakes of the same kind, etc)

- Dave


> On Apr 13, 2016 19:32, "David Blaikie" <dblaikie at gmail.com> wrote:
>
>> Was there a thread/discussion/context for this choice?
>>
>> On Wed, Apr 13, 2016 at 1:58 AM, Alexander Kornienko via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: alexfh
>>> Date: Wed Apr 13 03:58:52 2016
>>> New Revision: 266183
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=266183&view=rev
>>> Log:
>>> Don't use misc-unused-parameters check on LLVM.
>>>
>>> Modified:
>>>     llvm/trunk/.clang-tidy
>>>
>>> Modified: llvm/trunk/.clang-tidy
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/.clang-tidy?rev=266183&r1=266182&r2=266183&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/.clang-tidy (original)
>>> +++ llvm/trunk/.clang-tidy Wed Apr 13 03:58:52 2016
>>> @@ -1,4 +1,4 @@
>>> -Checks:
>>> '-*,clang-diagnostic-*,llvm-*,misc-*,readability-identifier-naming'
>>> +Checks:
>>> '-*,clang-diagnostic-*,llvm-*,misc-*,-misc-unused-parameters,readability-identifier-naming'
>>>  CheckOptions:
>>>    - key:             readability-identifier-naming.ClassCase
>>>      value:           CamelCase
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160414/44ac3745/attachment.html>