[LLVMdev] Any objections to turning on -Wunused-parameter? (and any other -Wunused-* that are off?)

David Blaikie dblaikie at gmail.com
Mon Jul 20 16:43:39 PDT 2015


On Mon, Jul 20, 2015 at 4:18 PM, Philip Reames <listmail at philipreames.com>
wrote:

>  I'd be very happy to see this turned on as a warning; I'd be very
> reluctant to see it turned on as a error.  During a lot of code cleanups,
> the intermediate state of "deleted, but dead args still passed in" is a
> very useful intermediate step.  It greatly simplifies review to be able to
> separate the meat of the semantic change from the "just deleting a bunch of
> dead args" patch.  I'd be very very reluctant to create a situation where
> the patch with the semantic change but without the argument cleanup won't
> pass all the build bots.
>

Yep - I recall Chandler expressing similar consternation when he & I have
discussed -Wunreachable-code so I was a bit surprised to see the suggestion.

But that said, in this case it doesn't seem too costly to comment out the
name of the parameter in the first patch - it is an extra change, but
doesn't require touching all the callers, etc. Then do the removal in a
follow up patch still.


>
> Philip
>
> (1 inline comment below as well)
>
> On 07/18/2015 02:48 AM, Chandler Carruth wrote:
>
> LLVM and Clang both have lots of objects that are passed through many
> different API boundaries. Things like AliasAnalysis in LLVM or Sema in
> Clang get threaded all over the place.
>
>  Over times, refactoring can often cause the parameters (or local
> variables, or member variables, etc) to become dead. If we notice this, we
> can often un-thread the interface through our APIs, sometimes even reducing
> coupling, etc.
>
>  But currently -Wunused-parameter is hard disabled. I've not checked to
> see if there are any other disabling of -Wunused-* variants, but I'd like
> to move the project toward firmly enabling them and being clean with them.
>
>  The technique I'd like to use is leaving out names of unused parameters,
> using a /*CommentedName*/ if there is a useful name, otherwise just
> omitting the name.
>
> Sorry, could you rephrase?  Not sure what you're getting at here.
>
>
>  All of this will require a reasonable amount of cleanup across the
> projects which I'm happy to do prior to flipping defaults around. Thoughts?
> Any concerns or objections?
>
>  (I'll ask the same question and ofter to either enact the cleanups or
> toggle the warning(s) back off for each of the less intertwined subprojects
> like LLD, LLDB, Polly, etc.)
>
>  -Chandler
>
>
> _______________________________________________
> LLVM Developers mailing listLLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20150720/5153beee/attachment.html>


More information about the llvm-dev mailing list