r195814 - Remove a whole lot of unused variables

Alp Toker alp at nuanti.com
Wed Nov 27 13:13:48 PST 2013


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.

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?

Alp.



>
> Pasi.
>

-- 
http://www.nuanti.com
the browser experts




More information about the cfe-commits mailing list