On 04/05/2014 06:37, Nico Weber wrote:
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<[email protected]> 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<[email protected]> wrote:
>>(Forgot to say: This is also PR18265.)
>>
>>On Sat, May 3, 2014 at 6:07 PM, Nico Weber<[email protected]> 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.
clang-unused-local-typedefs.diff
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) {
Nice. Small suggestion: I'd go for a default argument DC = 0, and if
(!DC) DC = D->getDeclContext() to keep the call signature simple here.
Alp.
+ 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;
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
