r195814 - Remove a whole lot of unused variables

[Alp Toker]

On 27/11/2013 22:11, Pasi Parviainen wrote:
> 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.

Sure, a feature that expands all the macros would go somewhere alongside 
clang-tidy/clang-modernize in the extra tools if anyone ever wanted to 
give it a shot. It's would be kind of the holy grail for dead code 
cleanups :-)

Alp.


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

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