[cfe-dev] [Patch] Diagnose unused declaration

[Douglas Gregor]

On Jan 5, 2009, at 8:37 PM, Sebastian Redl wrote:

> Douglas Gregor wrote:
>> Hello,
>>
>> On Jan 4, 2009, at 7:59 PM, EdB wrote:
>>
>>> There is place  for improvement but I want to be sure that
>>> information are at
>>> the right place before.
>>>
>>
>> A few comments on this iteration:
>>
>> In Sema::PopDeclContext(), the warning-generation code needs to be
>> much more picky about which declarations it complains about. For
>> example, I tried this C++ example:
>>
>> namespace N {
>>   int x;
>> }
>>
>> namespace N {
>>   int f() {
>>     return x;
>>   }
>> }
>>
>> and Clang produced 4 warnings: one each for x, f, __builtin_va_list,
>> and N:
>>
>> This option should not produce warnings about any of these, because:
>>
>> 	- x is the definition of a variable that can be referred to by other
>> translation units, so even if it isn't used within this translation
>> unit, we should not warn about it. Plus, "x" is used later on in the
>> program (in a different DeclContext for the namespace N). Note that
>> GCC's -Wunused-variable documentation explicitly says that it only
>> warns about local variables and non-constant static variables; we
>> should do something similar.
>>
>> 	- f is the definition of a function, which could also be referenced
>> by other translation units. Note that GCC's -Wunused-function only
>> complains about static functions (and, presumably, functions in
>> anonymous namespaces) because those aren't visible outside of the
>> translation unit.
>>
>> 	- __builtin_va_list wasn't written by the user, so we shouldn't
>> complain about it. Besides, do we even want to complain about unused
>> typedefs? Especially in C++, there are certainly reasons to write
>> typedefs even if they aren't used in the translation unit.
>>
>> 	- N is a namespace; do we ever want to warn about those?
>>
>> I see that UsedDeclaration grows throughout the processing of the
>> translation unit, because nothing is ever removed from the set (even
>> when it is no longer visible through name lookup). We'd like to avoid
>> that.
>>
> I think both problems could be solved together by, instead of keeping
> track of declarations that are used, keeping track of those that are  
> unused.
> In other words, the UsedDeclarations becomes UnusedDeclarations.  
> When a
> new declaration is encountered, it is added to the set if
> 1) it is function-local or TU-local and
> 2) it does not have the unused attribute.
> When a declaration is used, it is removed from the set. When a scope  
> is
> left, we check which of the declarations now going out of scope are
> still in the set and warn about those.


Great idea! This set should also be smaller than the UsedDeclarations  
set would be.

	- Doug