r195814 - Remove a whole lot of unused variables

Pasi Parviainen pasi.parviainen at iki.fi
Wed Nov 27 14:11:25 PST 2013


On 27.11.2013 23:13, Alp Toker wrote:
>
> On 27/11/2013 20:48, Pasi Parviainen wrote:
>> On 27.11.2013 21:51, Alp Toker wrote:
>>>
>>> On 27/11/2013 18:12, Pasi Parviainen wrote:
>>>> On 27.11.2013 7:22, Alp Toker wrote:
>>>>> Author: alp
>>>>> Date: Tue Nov 26 23:22:15 2013
>>>>> New Revision: 195814
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=195814&view=rev
>>>>> Log:
>>>>> Remove a whole lot of unused variables
>>>>>
>>>>> There are about 30 removed in this patch, generated by a new FixIt I
>>>>> haven't
>>>>> got round to submitting yet.
>>>>
>>>>> Modified: cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp?rev=195814&r1=195813&r2=195814&view=diff
>>>>>
>>>>>
>>>>> ==============================================================================
>>>>>
>>>>>
>>>>> --- cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
>>>>> (original)
>>>>> +++ cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp Tue Nov
>>>>> 26 23:22:15 2013
>>>>> @@ -32,7 +32,6 @@ using namespace llvm::opt;
>>>>>
>>>>>   static FrontendAction *CreateFrontendBaseAction(CompilerInstance
>>>>> &CI) {
>>>>>     using namespace clang::frontend;
>>>>> -  StringRef Action("unknown");
>>>>>
>>>>
>>>> This breaks the build if one or more of these macros are undefined:
>>>> CLANG_ENABLE_ARCMT, CLANG_ENABLE_STATIC_ANALYZER,
>>>> CLANG_ENABLE_REWRITER.
>>>>
>>>> Perhaps this variable should be under similar #if guard what can be
>>>> found later in the function for reporting error if disabled feature is
>>>> used. This would clarify intent of variable and avoid unused variable
>>>> warning.
>>>
>>> Hi Pasi,
>>>
>>> Thanks for noticing this! Fixed in r195872 along with a cast-to-void to
>>> suppress the unused variable warning.
>>>
>>> It's kind of unfortunate, I sat up waiting for email from the build
>>> servers to make sure this didn't break anything but never got notified.
>>> Feels like I get automated email about everyone's build breakage but not
>>> my own ;-)
>>
>> Nice! I did notice this with my local build with ARCMT and ANALYZER
>> disabled. If this was passed by automated builders, then I would
>> suspect that none of those are disabling one of those settings in
>> configuration (since those are enabled by default).
>
> Right, and realistically the builders are never going to test every
> permutation of the ENABLE flags, so for now we just have to be vigilant
> after landing cleanup patches.

Yeah, it is unrealistic expectation to test every permutation. At least 
test machinery could be extended to realistic expectations, like FreeBSD 
build depending all of those three options being disabled during early 
stages of FreeBSD world build. But then the question is, who has 
resources for it and is able to provide such a builder.

> It's actually a fascinating problem to solve if we could implement a
> dead code / unused variable checker that knows how to explode the
> preprocessor conditional stack, kind of the same way the static analyser
> tries every branch -- this is totally implementable for ifdefs that
> don't cross function scope boundaries. There's a research project in
> that, any takers?

This for sure is a interesting approach to the problem, but I fear it 
might be too expensive (and risky) for a general compilation pipeline.

> Alp.
>
>>
>> Pasi.
>>
>




More information about the cfe-commits mailing list