[patch] Implementing -Wunused-local-typedefs
Nico Weber
thakis at chromium.org
Sat May 3 22:37:24 PDT 2014
With one more setReferenced() call (in SemaCXXScopeSpec.cpp). This
version has successfully compiled several thousand files of chromium
(all that I've tried so far).
On Sat, May 3, 2014 at 6:35 PM, Nico Weber <thakis at chromium.org> wrote:
> 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.
-------------- next part --------------
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 207920)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -382,6 +382,7 @@
def UnusedConstVariable : DiagGroup<"unused-const-variable">;
def UnusedVariable : DiagGroup<"unused-variable",
[UnusedConstVariable]>;
+def UnusedLocalTypedefs : DiagGroup<"unused-local-typedefs">;
def UnusedPropertyIvar : DiagGroup<"unused-property-ivar">;
def UsedButMarkedUnused : DiagGroup<"used-but-marked-unused">;
def UserDefinedLiterals : DiagGroup<"user-defined-literals">;
@@ -502,7 +503,7 @@
[UnusedArgument, UnusedFunction, UnusedLabel,
// UnusedParameter, (matches GCC's behavior)
// UnusedMemberFunction, (clean-up llvm before enabling)
- UnusedPrivateField,
+ UnusedPrivateField, UnusedLocalTypedefs,
UnusedValue, UnusedVariable, UnusedPropertyIvar]>,
DiagCategory<"Unused Entity Issue">;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 207920)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -167,6 +167,8 @@
InGroup<UnusedParameter>, DefaultIgnore;
def warn_unused_variable : Warning<"unused variable %0">,
InGroup<UnusedVariable>, DefaultIgnore;
+def warn_unused_local_typedefs : Warning<"unused typedef %0">,
+ InGroup<UnusedLocalTypedefs>, DefaultIgnore;
def warn_unused_property_backing_ivar :
Warning<"ivar %0 which backs the property is not "
"referenced in this property's accessor">,
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 207920)
+++ include/clang/Sema/Sema.h (working copy)
@@ -3158,7 +3158,7 @@
/// DiagnoseUnusedExprResult - If the statement passed in is an expression
/// whose result is unused, warn.
void DiagnoseUnusedExprResult(const Stmt *S);
- void DiagnoseUnusedDecl(const NamedDecl *ND);
+ void DiagnoseUnusedDecl(const NamedDecl *ND, const DeclContext *DC);
/// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null
/// statement as a \p Body, and it is located on the same line.
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp (revision 207920)
+++ lib/Parse/ParseExprCXX.cpp (working copy)
@@ -426,8 +426,8 @@
if (Next.is(tok::coloncolon)) {
if (CheckForDestructor && GetLookAheadToken(2).is(tok::tilde) &&
- !Actions.isNonTypeNestedNameSpecifier(getCurScope(), SS, Tok.getLocation(),
- II, ObjectType)) {
+ !Actions.isNonTypeNestedNameSpecifier(
+ getCurScope(), SS, Tok.getLocation(), II, ObjectType)) {
*MayBePseudoDestructor = true;
return false;
}
Index: lib/Sema/SemaCXXScopeSpec.cpp
===================================================================
--- lib/Sema/SemaCXXScopeSpec.cpp (revision 207920)
+++ lib/Sema/SemaCXXScopeSpec.cpp (working copy)
@@ -614,6 +614,9 @@
}
}
+ if (TypedefDecl* TD = dyn_cast_or_null<TypedefDecl>(SD))
+ TD->setReferenced();
+
// If we're just performing this lookup for error-recovery purposes,
// don't extend the nested-name-specifier. Just return now.
if (ErrorRecoveryLookup)
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp (revision 207922)
+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -310,6 +310,7 @@
DiagnoseUseOfDecl(IIDecl, NameLoc);
T = Context.getTypeDeclType(TD);
+ TD->setReferenced();
// NOTE: avoid constructing an ElaboratedType(Loc) if this is a
// constructor or destructor name (in such a case, the scope specifier
@@ -813,6 +814,7 @@
NamedDecl *FirstDecl = (*Result.begin())->getUnderlyingDecl();
if (TypeDecl *Type = dyn_cast<TypeDecl>(FirstDecl)) {
DiagnoseUseOfDecl(Type, NameLoc);
+ Type->setReferenced();
QualType T = Context.getTypeDeclType(Type);
if (SS.isNotEmpty())
return buildNestedType(*this, SS, T, NameLoc);
@@ -1268,7 +1270,8 @@
UnusedFileScopedDecls.push_back(D);
}
-static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
+static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D,
+ const DeclContext *DC) {
if (D->isInvalidDecl())
return false;
@@ -1278,10 +1281,15 @@
if (isa<LabelDecl>(D))
return true;
+
+ if (!DC->isFunctionOrMethod())
+ return false;
+
+ if (isa<TypedefDecl>(D))
+ return true;
// White-list anything that isn't a local variable.
- if (!isa<VarDecl>(D) || isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D) ||
- !D->getDeclContext()->isFunctionOrMethod())
+ if (!isa<VarDecl>(D) || isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D))
return false;
// Types of valid local variables should be complete, so this should succeed.
@@ -1346,8 +1354,14 @@
/// DiagnoseUnusedDecl - Emit warnings about declarations that are not used
/// unless they are marked attr(unused).
-void Sema::DiagnoseUnusedDecl(const NamedDecl *D) {
- if (!ShouldDiagnoseUnusedDecl(D))
+void Sema::DiagnoseUnusedDecl(const NamedDecl *D, const DeclContext *DC) {
+ if (DC->isFunctionOrMethod())
+ if (const RecordDecl *RD = dyn_cast<RecordDecl>(D))
+ for (auto *TmpD : RD->decls())
+ if (isa<TypedefDecl>(TmpD) || isa<RecordDecl>(TmpD))
+ DiagnoseUnusedDecl(cast<NamedDecl>(TmpD), DC);
+
+ if (!ShouldDiagnoseUnusedDecl(D, DC))
return;
FixItHint Hint;
@@ -1358,6 +1372,8 @@
DiagID = diag::warn_unused_exception_param;
else if (isa<LabelDecl>(D))
DiagID = diag::warn_unused_label;
+ else if (isa<TypedefDecl>(D)) // TODO: Warn on unused c++11 aliases too?
+ DiagID = diag::warn_unused_local_typedefs;
else
DiagID = diag::warn_unused_variable;
@@ -1389,7 +1405,7 @@
// Diagnose unused variables in this scope.
if (!S->hasUnrecoverableErrorOccurred())
- DiagnoseUnusedDecl(D);
+ DiagnoseUnusedDecl(D, D->getDeclContext());
// If this was a forward reference to a label, verify it was defined.
if (LabelDecl *LD = dyn_cast<LabelDecl>(D))
Index: lib/Sema/SemaStmtAsm.cpp
===================================================================
--- lib/Sema/SemaStmtAsm.cpp (revision 207920)
+++ lib/Sema/SemaStmtAsm.cpp (working copy)
@@ -441,8 +441,10 @@
NamedDecl *FoundDecl = BaseResult.getFoundDecl();
if (VarDecl *VD = dyn_cast<VarDecl>(FoundDecl))
RT = VD->getType()->getAs<RecordType>();
- else if (TypedefDecl *TD = dyn_cast<TypedefDecl>(FoundDecl))
+ else if (TypedefDecl *TD = dyn_cast<TypedefDecl>(FoundDecl)) {
+ TD->setReferenced();
RT = TD->getUnderlyingType()->getAs<RecordType>();
+ }
if (!RT)
return true;
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp (revision 207920)
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp (working copy)
@@ -3651,7 +3651,7 @@
if (!NewVar->isInvalidDecl() &&
NewVar->getDeclContext()->isFunctionOrMethod() && !NewVar->isUsed() &&
OldVar->getType()->isDependentType())
- DiagnoseUnusedDecl(NewVar);
+ DiagnoseUnusedDecl(NewVar, NewVar->getDeclContext());
}
/// \brief Instantiate the initializer of a variable.
Index: test/SemaCXX/implicit-exception-spec.cpp
===================================================================
--- test/SemaCXX/implicit-exception-spec.cpp (revision 207920)
+++ test/SemaCXX/implicit-exception-spec.cpp (working copy)
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -verify -std=c++11 -Wall %s
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -verify -std=c++11 -Wall -Wno-unused-local-typedefs %s
template<bool b> struct ExceptionIf { static int f(); };
template<> struct ExceptionIf<false> { typedef int f; };
Index: test/SemaCXX/warn-unused-filescoped.cpp
===================================================================
--- test/SemaCXX/warn-unused-filescoped.cpp (revision 207920)
+++ test/SemaCXX/warn-unused-filescoped.cpp (working copy)
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -Wno-c++11-extensions -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -Wno-unused-local-typedefs -Wno-c++11-extensions -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -Wno-unused-local-typedefs -std=c++11 %s
#ifdef HEADER
Index: test/SemaCXX/warn-unused-local-typedefs.cpp
===================================================================
--- test/SemaCXX/warn-unused-local-typedefs.cpp (revision 0)
+++ test/SemaCXX/warn-unused-local-typedefs.cpp (working copy)
@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 -Wunused-local-typedefs -fasm-blocks -verify -std=c++11 %s
+
+struct S {
+ typedef int Foo; // no diag
+};
+
+namespace N {
+ typedef int Foo; // no diag
+ typedef int Foo2; // no diag
+}
+
+template <class T> class Vec {};
+
+typedef int global_foo; // no diag (?)
+
+void f() {
+ typedef int foo0; // expected-warning {{unused typedef 'foo0'}}
+
+ typedef int foo1 __attribute__((unused)); // no diag
+
+ typedef int foo2;
+ {
+ typedef int foo2; // expected-warning {{unused typedef 'foo2'}}
+ }
+ typedef foo2 foo3; // expected-warning {{unused typedef 'foo3'}}
+
+ typedef int foo2_2; // expected-warning {{unused typedef 'foo2_2'}}
+ {
+ typedef int foo2_2;
+ typedef foo2_2 foo3_2; // expected-warning {{unused typedef 'foo3_2'}}
+ }
+
+ typedef int foo4;
+ foo4 the_thing;
+
+ typedef int* foo5;
+ typedef foo5* foo6; // no diag
+ foo6 *myptr;
+
+ struct S2 {
+ typedef int Foo, Foo2; // expected-warning {{unused typedef 'Foo2'}}
+
+ struct Deeper {
+ typedef int DeepFoo; // expected-warning {{unused typedef 'DeepFoo'}}
+ };
+ };
+
+ S2::Foo s2foo;
+
+ typedef struct {} foostruct; // expected-warning {{unused typedef 'foostruct'}}
+
+ typedef struct {} foostruct2; // no diag
+ foostruct2 fs2;
+
+ typedef int vecint; // no diag
+ Vec<vecint> v;
+
+ N::Foo nfoo;
+
+ typedef int cint; // no diag
+ constexpr int kFoo = (int)(cint)5;
+
+ typedef struct {
+ int a;
+ int b;
+ } A; // no diag
+ __asm mov eax, [eax].A.b
+}
+
+int printf(char const *, ...);
+
+void test() {
+ typedef signed long int superint; // no diag
+ printf("%f", (superint) 42);
+
+ typedef signed long int superint2; // no diag
+ printf("%f", static_cast<superint2>(42));
+}
+
+template<typename T>
+void g() {
+ typedef T foo; // expected-warning{{unused typedef 'foo'}}
+}
+
+void t5() {
+ class A {
+ };
+ typedef A tA;
+ typedef A tA2; // expected-warning{{unused typedef 'tA2'}}
+ class B {
+ friend tA;
+ };
+}
+
+template<class T>
+struct V {
+ typedef int iterator;
+};
+
+void chain() {
+ typedef V<int> v; // no diag
+ v::iterator it;
+}
More information about the cfe-commits
mailing list