[patch] Implementing -Wunused-local-typedefs

[Nico Weber]

About point 2: The patch currently incorrectly warns on the "v" typedef in:

  template <class T> struct V { typedef int iterator; };
  void f() {
    typedef V<int> v;
    v::iterator it;
  }

So there's at least one more place where I need to insert a
setReferenced() I'll send an updated version once I found where, but
I'm thankful for comments on the overall approach in the meantime too.

On Sat, May 3, 2014 at 6:08 PM, Nico Weber <thakis at chromium.org> wrote:
> (Forgot to say: This is also PR18265.)
>
> On Sat, May 3, 2014 at 6:07 PM, Nico Weber <thakis at chromium.org> wrote:
>> Hi,
>>
>> gcc has a warning -Wunused-local-typedefs that warns on unused local
>> typedefs. Inspired by r207870, I thought it'd be nice if clang had
>> this warning too. This warning requires the following three things:
>>
>>
>> 1.) A bit to track if a typedef has been referenced.
>>
>> Decl already has isUsed and isReferenced, but they don't seem to be
>> used for TypedefDecls, so I'm just using isReferenced on TypedefDecls
>> for this.
>>
>>
>> 2.) Setting that bit on typedefs that are used.
>>
>> The three strategies I can think of:
>> a.) Do this in Sema::DiagnoseUseOfDecl(), this seems to be already
>> called for decls all over the place, and an "if isa<TypedefDecl>(D)
>> D->setReferenced()" seems to do the trick.
>> b.) Do this in LookupName
>> c.) Do this explicitly in the places where it's actually necessary.
>>
>> The attached patch goes with the last approach. The three places I
>> found where it's needed are Sema::getTypeName(), Sema::ClassifyName(),
>> and Sema::LookupInlineAsmField(). The first two are called by the
>> parser for almost everything, while the third is called for references
>> to structs from MS inline assembly. That last one is a bit scary as it
>> means it's possible that there are other places this is needed that I
>> missed.
>>
>>
>> 3.) A way to check all typedefs in a scope when that scope ends.
>>
>> I added this to Sema::ActOnPopScope() which is where the
>> unused-variable and unused-label warnings are created. This works
>> well, but to catch the unused local typedef in
>>
>>   {
>>     struct A {
>>       struct B { typedef int SoDeep; };
>>     };
>>   }
>>
>> it means that that code now needs to recurse into all RecordDecl in a
>> scope, which it didn't need to do previously.
>>
>>
>> Let me know how badly I've chosen in each instance :-)
>>
>> Thanks,
>> Nico
>>
>> ps: If this goes in, a follow-up question is if this should warn on
>> unused C++11 type aliases too – it'd just require
>> s/TypedefDecl/TypedefNameDecl/ in two places in the patch, so it
>> should be an easy follow-up.