[patch] Implementing -Wunused-local-typedefs
Nico Weber
thakis at chromium.org
Sat Aug 9 17:46:06 PDT 2014
On Wed, Aug 6, 2014 at 7:05 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> + /// \brief Set containing all declared private fields that are not used.
> typedef llvm::SmallSetVector<const NamedDecl*, 16> NamedDeclSetType;
> -
> - /// \brief Set containing all declared private fields that are not used.
> NamedDeclSetType UnusedPrivateFields;
>
> Doesn't this make Doxygen attach the documentation to the wrong thing?
>
Fixed.
> + /// \brief Set containing all typedefs that are likely unused.
> + llvm::SmallSetVector<const TypedefNameDecl *, 4>
> + UnusedLocalTypedefNameCandidates;
>
> You don't seem to have any serialization/deserialization code for this; I
> don't think this is doing quite the right thing in all the AST-file cases.
> Preambles and PCH should behave exactly like we'd actually included the
> file, so in those cases you'd need to serialize/deserialize. For modules,
> you probably want to issue the diagnostic while building the module (you
> currently won't, because the emission code is after the end of the module).
>
Added serialization code. For modules, the diagnostics are emitted when the
module is used, not when it's build (this seems to match the other unused
warnings).
Are there good example for how to test the serialization bits? -verify
doesn't work well with .ast files as far as I can tell. (I guess I could
have a similar test that uses FileCheck instead of -verify, but that's kind
of lame.)
>
> I've thought about the C++14 deduced return type issue a bit more, and I
> think there's a better way of handling it: when we deduce a return type for
> a function, if the type involves any local class types, mark all of their
> members as referenced. (Those members have potentially escaped our
> analysis, even if they're not used in this translation unit.) We could be
> smarter here and do different things if the function is not an external
> linkage inline function (delay until end of TU, perhaps), but I don't think
> it's an important case. The current warning would have false positives if
> an escaped type's typedefs are used in one TU but not another.
>
Done, including the internal linkage thing you suggested. I also don't mark
private typedefs as referenced. I kept the "delay everything until the end"
for now, since it's needed in the general case, and having just one
codepath that emits this warning seems nice.
> One other thing: we only need to delay diagnosing unused local typedefs if
> they're local class members, I think -- otherwise, we should see the use
> before they get popped. Even then, the only way we can get a delayed use is
> if they're involved in a template argument in some way. We could possibly
> detect that case and treat them as escaped, but perhaps that would be too
> expensive.
>
> Presumably we can apply the unused-local-class-member-typedef diagnostic
> mechanism to cover *all* local class members?
>
That's a good idea for a follow-up, yes.
>
>
> On Wed, Aug 6, 2014 at 6:08 PM, Nico Weber <thakis at chromium.org> wrote:
>
>> ping
>>
>>
>> On Sun, Jul 27, 2014 at 8:36 PM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> This now also warns on
>>>
>>> template <class T>
>>> void template_fun(T t) {
>>> struct S2 {
>>> typedef int Foo1;
>>> };
>>> }
>>> void template_fun_user() {
>>> template_fun(3);
>>> }
>>>
>>> That is, it fixes the FIXME mentioned in the last post (by checking for
>>> unused typedefs in TemplateDeclInstantiator::VisitCXXRecordDecl after the
>>> record decl has been instantiated). I also
>>> changed ShouldDiagnoseUnusedDecl() to not take a new parameter but instead
>>> compute WithinFunction locally like you suggested.
>>>
>>> (I also removed a redundant IsUsed() check
>>> in Sema::BuildVariableInstantiation() – DiagnoseUnusedDecl() checks that
>>> already.)
>>>
>>> I'll stop futzing with this now; I think this is read for a look. (Not
>>> urgent at all, of course.)
>>>
>>>
>>> On Sun, Jul 27, 2014 at 5:13 PM, Nico Weber <thakis at chromium.org> wrote:
>>>
>>>> Now gets this right:
>>>>
>>>> template <class T>
>>>> void template_fun(T t) {
>>>> typename T::Foo s3foo;
>>>> }
>>>> void template_fun_user() {
>>>> struct Local {
>>>> typedef int Foo; // no-diag
>>>> typedef int Bar; // expected-warning {{unused typedef 'Bar'}}
>>>> } p;
>>>> template_fun(p);
>>>> }
>>>>
>>>> It also no longer warns on S::Foo in:
>>>>
>>>> template<class T>
>>>> void tf(T t) {
>>>> struct S {
>>>> typedef int Foo;
>>>> typedef int Bar;
>>>> };
>>>> S::Foo f;
>>>> }
>>>>
>>>> (It stopped warning on S::Bar too – there's a FIXME in the test that
>>>> explains why and how to fix that.)
>>>>
>>>>
>>>>
>>>> On Sat, Jul 26, 2014 at 9:26 PM, Nico Weber <thakis at chromium.org>
>>>> wrote:
>>>>
>>>>> I'm delighted to announce that, after almost two months of hard work,
>>>>> a new patch set is available for review!
>>>>>
>>>>> With these tantalizing features and fixes:
>>>>> * TypedefNameDecls everywhere
>>>>> * MarkAnyDeclReferenced() used throughout
>>>>> * Instead of immediately emitting a diag, the TypedefNameDecls are
>>>>> stashed in a set and the diagnostics for things in the set are emitted at
>>>>> end-of-TU (except for TypedefNameDecls that have been referenced since then)
>>>>> * It's now called -Wunused-local-typedef in singular, with an alias
>>>>> for the gcc spelling
>>>>> * More tests
>>>>>
>>>>> On Tue, May 6, 2014 at 6:01 PM, Richard Smith <richard at metafoo.co.uk>
>>>>> wrote:
>>>>>
>>>>>> def UnusedConstVariable : DiagGroup<"unused-const-variable">;
>>>>>> def UnusedVariable : DiagGroup<"unused-variable",
>>>>>> [UnusedConstVariable]>;
>>>>>> +def UnusedLocalTypedefs : DiagGroup<"unused-local-typedefs">;
>>>>>> def UnusedPropertyIvar : DiagGroup<"unused-property-ivar">;
>>>>>>
>>>>>> I'm a little uncomfortable about this flag being plural and the
>>>>>> others being singular. GCC compatible, you say? =(
>>>>>>
>>>>>>
>>>>>> +def warn_unused_local_typedefs : Warning<"unused typedef %0">,
>>>>>>
>>>>>> This one should definitely be singular.
>>>>>>
>>>>>> Reformatting of lib/Parse/ParseExprCXX.cpp looks unrelated. Feel free
>>>>>> to commit that separately.
>>>>>>
>>>>>> + if (TypedefDecl* TD = dyn_cast_or_null<TypedefDecl>(SD))
>>>>>>
>>>>>> * should go on the right. Also, use 'auto' for a variable initialized
>>>>>> from cast/dyn_cast/etc.
>>>>>>
>>>>>> Rather than calling setReferenced directly, please call
>>>>>> Sema::MarkAnyDeclReferenced.
>>>>>>
>>>>>> +static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D,
>>>>>> + const DeclContext *DC) {
>>>>>> [...]
>>>>>> + if (!DC->isFunctionOrMethod())
>>>>>> + return false;
>>>>>>
>>>>>> Please document what 'DC' is here; it's really not obvious. But it
>>>>>> seems to me that you can replace DC here and below with just a bool
>>>>>> WithinFunction.
>>>>>>
>>>>>
>>>>> Done.
>>>>>
>>>>>
>>>>>> You can also remove it entirely and check whether
>>>>>> D->getDeclContext() is a function, method, or local class.
>>>>>>
>>>>>> +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);
>>>>>>
>>>>>> This would make more sense as a layer between ActOnPopScope and
>>>>>> DiagnoseUnusedDecl.
>>>>>>
>>>>>
>>>>> Done.
>>>>>
>>>>>
>>>>>> A comment explaining why we only do this within a function or method
>>>>>> would help ("within a function or method, we defer these checks until the
>>>>>> end of the surrounding scope").
>>>>>>
>>>>>> Also, this isn't necessarily correct in C++14, since a local type's
>>>>>> members can be first used once we've already left the function:
>>>>>>
>>>>>> auto f() {
>>>>>> struct S { typedef int t; };
>>>>>> return S();
>>>>>> }
>>>>>> auto x = f();
>>>>>> decltype(x)::t y;
>>>>>>
>>>>>> What happens if the local type is used as an argument to a function
>>>>>> template, which in turn uses the typedef? Can we really diagnose this
>>>>>> before end-of-TU?
>>>>>>
>>>>>
>>>>> Done.
>>>>>
>>>>>
>>>>>> + else if (isa<TypedefDecl>(D)) // TODO: Warn on unused c++11
>>>>>> aliases too?
>>>>>> + DiagID = diag::warn_unused_local_typedefs;
>>>>>>
>>>>>> Yes, please change this to TypedefNameDecl (throughout the patch). It
>>>>>> doesn't make sense to handle these differently, since one can be a
>>>>>> redeclaration of the other.
>>>>>>
>>>>>
>>>>> Done.
>>>>>
>>>>>
>>>>>>
>>>>>> 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());
>>>>>>
>>>>>> Hmm, why do we diagnose unused decls from template instantiation at
>>>>>> all?
>>>>>>
>>>>>
>>>>> r103362 added this, r133580 tweaked it some. Can you think of an
>>>>> example where a template-local typedef is unused only in some
>>>>> instantiations of that template?
>>>>>
>>>>>
>>>>>>
>>>>>> On Sat, May 3, 2014 at 10:37 PM, Nico Weber <thakis at chromium.org>
>>>>>> 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 <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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140809/34e6369e/attachment.html>
-------------- next part --------------
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td (revision 214046)
+++ include/clang/Basic/DiagnosticGroups.td (working copy)
@@ -401,6 +401,7 @@
def UnusedConstVariable : DiagGroup<"unused-const-variable">;
def UnusedVariable : DiagGroup<"unused-variable",
[UnusedConstVariable]>;
+def UnusedLocalTypedef : DiagGroup<"unused-local-typedef">;
def UnusedPropertyIvar : DiagGroup<"unused-property-ivar">;
def UsedButMarkedUnused : DiagGroup<"used-but-marked-unused">;
def UserDefinedLiterals : DiagGroup<"user-defined-literals">;
@@ -523,7 +524,7 @@
[UnusedArgument, UnusedFunction, UnusedLabel,
// UnusedParameter, (matches GCC's behavior)
// UnusedMemberFunction, (clean-up llvm before enabling)
- UnusedPrivateField,
+ UnusedPrivateField, UnusedLocalTypedef,
UnusedValue, UnusedVariable, UnusedPropertyIvar]>,
DiagCategory<"Unused Entity Issue">;
@@ -617,6 +618,8 @@
[IntConversion]>; // -Wint-conversions = -Wint-conversion
def : DiagGroup<"vector-conversions",
[VectorConversion]>; // -Wvector-conversions = -Wvector-conversion
+def : DiagGroup<"unused-local-typedefs", [UnusedLocalTypedef]>;
+ // -Wunused-local-typedefs = -Wunused-local-typedef
// A warning group for warnings that we want to have on by default in clang,
// but which aren't on by default in GCC.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 214046)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -175,6 +175,9 @@
InGroup<UnusedParameter>, DefaultIgnore;
def warn_unused_variable : Warning<"unused variable %0">,
InGroup<UnusedVariable>, DefaultIgnore;
+def warn_unused_local_typedef : Warning<
+ "unused %select{typedef|type alias}0 %1">,
+ InGroup<UnusedLocalTypedef>, 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/ExternalSemaSource.h
===================================================================
--- include/clang/Sema/ExternalSemaSource.h (revision 214046)
+++ include/clang/Sema/ExternalSemaSource.h (working copy)
@@ -20,6 +20,10 @@
#include "llvm/ADT/MapVector.h"
#include <utility>
+namespace llvm {
+template <class T, unsigned n> class SmallSetVector;
+}
+
namespace clang {
class CXXConstructorDecl;
@@ -132,6 +136,15 @@
/// introduce the same declarations repeatedly.
virtual void ReadDynamicClasses(SmallVectorImpl<CXXRecordDecl *> &Decls) {}
+ /// \brief Read the set of potentially unused typedefs known to the source.
+ ///
+ /// The external source should append its own potentially unused local
+ /// typedefs to the given vector of declarations. Note that this routine may
+ /// be invoked multiple times; the external source should take care not to
+ /// introduce the same declarations repeatedly.
+ virtual void ReadUnusedLocalTypedefNameCandidates(
+ llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) {};
+
/// \brief Read the set of locally-scoped external declarations known to the
/// external Sema source.
///
Index: include/clang/Sema/MultiplexExternalSemaSource.h
===================================================================
--- include/clang/Sema/MultiplexExternalSemaSource.h (revision 214046)
+++ include/clang/Sema/MultiplexExternalSemaSource.h (working copy)
@@ -282,6 +282,15 @@
/// introduce the same declarations repeatedly.
void ReadDynamicClasses(SmallVectorImpl<CXXRecordDecl*> &Decls) override;
+ /// \brief Read the set of potentially unused typedefs known to the source.
+ ///
+ /// The external source should append its own potentially unused local
+ /// typedefs to the given vector of declarations. Note that this routine may
+ /// be invoked multiple times; the external source should take care not to
+ /// introduce the same declarations repeatedly.
+ void ReadUnusedLocalTypedefNameCandidates(
+ llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) override;
+
/// \brief Read the set of locally-scoped extern "C" declarations known to the
/// external Sema source.
///
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h (revision 214046)
+++ include/clang/Sema/Sema.h (working copy)
@@ -390,6 +390,10 @@
/// \brief Set containing all declared private fields that are not used.
NamedDeclSetType UnusedPrivateFields;
+ /// \brief Set containing all typedefs that are likely unused.
+ llvm::SmallSetVector<const TypedefNameDecl *, 4>
+ UnusedLocalTypedefNameCandidates;
+
typedef llvm::SmallPtrSet<const CXXRecordDecl*, 8> RecordDeclSetTy;
/// PureVirtualClassDiagSet - a set of class declarations which we have
@@ -3187,6 +3191,7 @@
/// DiagnoseUnusedExprResult - If the statement passed in is an expression
/// whose result is unused, warn.
void DiagnoseUnusedExprResult(const Stmt *S);
+ void DiagnoseUnusedNestedTypedefs(const RecordDecl *D);
void DiagnoseUnusedDecl(const NamedDecl *ND);
/// Emit \p DiagID if statement located on \p StmtLoc has a suspicious null
Index: include/clang/Serialization/ASTBitCodes.h
===================================================================
--- include/clang/Serialization/ASTBitCodes.h (revision 214046)
+++ include/clang/Serialization/ASTBitCodes.h (working copy)
@@ -545,7 +545,10 @@
LATE_PARSED_TEMPLATE = 50,
/// \brief Record code for \#pragma optimize options.
- OPTIMIZE_PRAGMA_OPTIONS = 51
+ OPTIMIZE_PRAGMA_OPTIONS = 51,
+
+ /// \brief Record code for potentially unused local typedef names.
+ UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES = 52,
};
/// \brief Record types used within a source manager block.
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h (revision 214046)
+++ include/clang/Serialization/ASTReader.h (working copy)
@@ -748,6 +748,11 @@
/// at the end of the TU, in which case it directs CodeGen to emit the VTable.
SmallVector<uint64_t, 16> DynamicClasses;
+ /// \brief The IDs of all potentially unused typedef names in the chain.
+ ///
+ /// Sema tracks these to emit warnings.
+ SmallVector<uint64_t, 16> UnusedLocalTypedefNameCandidates;
+
/// \brief The IDs of the declarations Sema stores directly.
///
/// Sema tracks a few important decls, such as namespace std, directly.
@@ -1772,6 +1777,9 @@
void ReadDynamicClasses(SmallVectorImpl<CXXRecordDecl *> &Decls) override;
+ void ReadUnusedLocalTypedefNameCandidates(
+ llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) override;
+
void ReadLocallyScopedExternCDecls(
SmallVectorImpl<NamedDecl *> &Decls) override;
Index: lib/Sema/MultiplexExternalSemaSource.cpp
===================================================================
--- lib/Sema/MultiplexExternalSemaSource.cpp (revision 214046)
+++ lib/Sema/MultiplexExternalSemaSource.cpp (working copy)
@@ -242,6 +242,12 @@
Sources[i]->ReadDynamicClasses(Decls);
}
+void MultiplexExternalSemaSource::ReadUnusedLocalTypedefNameCandidates(
+ llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) {
+ for(size_t i = 0; i < Sources.size(); ++i)
+ Sources[i]->ReadUnusedLocalTypedefNameCandidates(Decls);
+}
+
void MultiplexExternalSemaSource::ReadLocallyScopedExternCDecls(
SmallVectorImpl<NamedDecl*> &Decls) {
for(size_t i = 0; i < Sources.size(); ++i)
Index: lib/Sema/Sema.cpp
===================================================================
--- lib/Sema/Sema.cpp (revision 214046)
+++ lib/Sema/Sema.cpp (working copy)
@@ -821,6 +821,18 @@
if (ExternalSource)
ExternalSource->ReadUndefinedButUsed(UndefinedButUsed);
checkUndefinedButUsed(*this);
+
+ if (!Diags.isIgnored(diag::warn_unused_local_typedef, SourceLocation())) {
+ if (ExternalSource)
+ ExternalSource->ReadUnusedLocalTypedefNameCandidates(
+ UnusedLocalTypedefNameCandidates);
+ for (const TypedefNameDecl *TD : UnusedLocalTypedefNameCandidates) {
+ if (TD->isReferenced())
+ continue;
+ Diag(TD->getLocation(), diag::warn_unused_local_typedef)
+ << isa<TypeAliasDecl>(TD) << TD->getDeclName();
+ }
+ }
}
if (!Diags.isIgnored(diag::warn_unused_private_field, SourceLocation())) {
Index: lib/Sema/SemaCXXScopeSpec.cpp
===================================================================
--- lib/Sema/SemaCXXScopeSpec.cpp (revision 214046)
+++ lib/Sema/SemaCXXScopeSpec.cpp (working copy)
@@ -612,6 +612,9 @@
}
}
+ if (auto *TD = dyn_cast_or_null<TypedefNameDecl>(SD))
+ MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
+
// 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 214408)
+++ lib/Sema/SemaDecl.cpp (working copy)
@@ -377,6 +377,7 @@
DiagnoseUseOfDecl(IIDecl, NameLoc);
T = Context.getTypeDeclType(TD);
+ MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
// NOTE: avoid constructing an ElaboratedType(Loc) if this is a
// constructor or destructor name (in such a case, the scope specifier
@@ -925,6 +926,7 @@
NamedDecl *FirstDecl = (*Result.begin())->getUnderlyingDecl();
if (TypeDecl *Type = dyn_cast<TypeDecl>(FirstDecl)) {
DiagnoseUseOfDecl(Type, NameLoc);
+ MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
QualType T = Context.getTypeDeclType(Type);
if (SS.isNotEmpty())
return buildNestedType(*this, SS, T, NameLoc);
@@ -1392,10 +1394,21 @@
if (isa<LabelDecl>(D))
return true;
+
+ // Except for labels, we only care about unused decls that are local to
+ // functions.
+ bool WithinFunction = D->getDeclContext()->isFunctionOrMethod();
+ if (const auto *R = dyn_cast<CXXRecordDecl>(D->getDeclContext()))
+ WithinFunction =
+ WithinFunction || (R->isLocalClass() && !R->isDependentType());
+ if (!WithinFunction)
+ return false;
+
+ if (isa<TypedefNameDecl>(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.
@@ -1458,11 +1471,30 @@
return;
}
+void Sema::DiagnoseUnusedNestedTypedefs(const RecordDecl *D) {
+ if (D->getTypeForDecl()->isDependentType())
+ return;
+
+ for (auto *TmpD : D->decls()) {
+ if (const auto* T = dyn_cast<TypedefNameDecl>(TmpD))
+ DiagnoseUnusedDecl(T);
+ else if(const auto *R = dyn_cast<RecordDecl>(TmpD))
+ DiagnoseUnusedNestedTypedefs(R);
+ }
+}
+
/// DiagnoseUnusedDecl - Emit warnings about declarations that are not used
/// unless they are marked attr(unused).
void Sema::DiagnoseUnusedDecl(const NamedDecl *D) {
if (!ShouldDiagnoseUnusedDecl(D))
return;
+
+ if (auto* TD = dyn_cast<TypedefNameDecl>(D)) {
+ // typedefs can be referenced later on, so the diagnostics are emitted
+ // at end-of-translation-unit.
+ UnusedLocalTypedefNameCandidates.insert(TD);
+ return;
+ }
FixItHint Hint;
GenerateFixForUnusedDecl(D, Context, Hint);
@@ -1502,8 +1534,11 @@
if (!D->getDeclName()) continue;
// Diagnose unused variables in this scope.
- if (!S->hasUnrecoverableErrorOccurred())
+ if (!S->hasUnrecoverableErrorOccurred()) {
DiagnoseUnusedDecl(D);
+ if (const auto *RD = dyn_cast<RecordDecl>(D))
+ DiagnoseUnusedNestedTypedefs(RD);
+ }
// If this was a forward reference to a label, verify it was defined.
if (LabelDecl *LD = dyn_cast<LabelDecl>(D))
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp (revision 214046)
+++ lib/Sema/SemaStmt.cpp (working copy)
@@ -19,6 +19,7 @@
#include "clang/AST/EvaluatedExprVisitor.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtObjC.h"
#include "clang/AST/TypeLoc.h"
@@ -2727,6 +2728,40 @@
return Result;
}
+namespace {
+/// \brief Marks all typedefs in all local classes in a type referenced.
+///
+/// In a function like
+/// auto f() {
+/// struct S { typedef int a; };
+/// return S;
+/// }
+///
+/// the local type escapes and could be referenced in some TUs but not in
+/// others. Pretend that all local typedefs are always references, to not warn
+/// on this. This isn't necessary if f has internal linkage, or the typedef
+/// is private.
+class LocalTypedefNameReferencer
+ : public RecursiveASTVisitor<LocalTypedefNameReferencer> {
+public:
+ LocalTypedefNameReferencer(Sema &S) : S(S) {}
+ bool VisitRecordType(const RecordType *RT);
+private:
+ Sema &S;
+};
+bool LocalTypedefNameReferencer::VisitRecordType(const RecordType *RT) {
+ auto *R = dyn_cast<CXXRecordDecl>(RT->getDecl());
+ if (!R || !R->isLocalClass() || !R->isLocalClass()->isExternallyVisible() ||
+ R->isDependentType())
+ return true;
+ for (auto *TmpD : R->decls())
+ if (auto *T = dyn_cast<TypedefNameDecl>(TmpD))
+ if (T->getAccess() != AS_private)
+ S.MarkAnyDeclReferenced(T->getLocation(), T, /*OdrUse=*/false);
+ return true;
+}
+}
+
/// Deduce the return type for a function from a returned expression, per
/// C++1y [dcl.spec.auto]p6.
bool Sema::DeduceFunctionTypeFromReturnExpr(FunctionDecl *FD,
@@ -2772,6 +2807,11 @@
if (DAR != DAR_Succeeded)
return true;
+
+ // If a local type is part of the returned type, mark its fields as
+ // referenced.
+ LocalTypedefNameReferencer Referencer(*this);
+ Referencer.TraverseType(RetExpr->getType());
} else {
// In the case of a return with no operand, the initializer is considered
// to be void().
Index: lib/Sema/SemaStmtAsm.cpp
===================================================================
--- lib/Sema/SemaStmtAsm.cpp (revision 214046)
+++ lib/Sema/SemaStmtAsm.cpp (working copy)
@@ -443,8 +443,10 @@
NamedDecl *FoundDecl = BaseResult.getFoundDecl();
if (VarDecl *VD = dyn_cast<VarDecl>(FoundDecl))
RT = VD->getType()->getAs<RecordType>();
- else if (TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(FoundDecl))
+ else if (TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(FoundDecl)) {
+ MarkAnyDeclReferenced(TD->getLocation(), TD, /*OdrUse=*/false);
RT = TD->getUnderlyingType()->getAs<RecordType>();
+ }
else if (TypeDecl *TD = dyn_cast<TypeDecl>(FoundDecl))
RT = TD->getTypeForDecl()->getAs<RecordType>();
if (!RT)
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp (revision 214046)
+++ lib/Sema/SemaTemplate.cpp (working copy)
@@ -7941,6 +7941,7 @@
if (TypeDecl *Type = dyn_cast<TypeDecl>(Result.getFoundDecl())) {
// We found a type. Build an ElaboratedType, since the
// typename-specifier was just sugar.
+ MarkAnyDeclReferenced(Type->getLocation(), Type, /*OdrUse=*/false);
return Context.getElaboratedType(ETK_Typename,
QualifierLoc.getNestedNameSpecifier(),
Context.getTypeDeclType(Type));
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp (revision 214046)
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp (working copy)
@@ -1191,6 +1191,9 @@
SemaRef.InstantiateClassMembers(D->getLocation(), Record, TemplateArgs,
TSK_ImplicitInstantiation);
}
+
+ SemaRef.DiagnoseUnusedNestedTypedefs(Record);
+
return Record;
}
@@ -3651,7 +3654,7 @@
// Diagnose unused local variables with dependent types, where the diagnostic
// will have been deferred.
if (!NewVar->isInvalidDecl() &&
- NewVar->getDeclContext()->isFunctionOrMethod() && !NewVar->isUsed() &&
+ NewVar->getDeclContext()->isFunctionOrMethod() &&
OldVar->getType()->isDependentType())
DiagnoseUnusedDecl(NewVar);
}
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp (revision 214046)
+++ lib/Serialization/ASTReader.cpp (working copy)
@@ -3310,6 +3310,12 @@
}
OptimizeOffPragmaLocation = ReadSourceLocation(F, Record[0]);
break;
+
+ case UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES:
+ for (unsigned I = 0, N = Record.size(); I != N; ++I)
+ UnusedLocalTypedefNameCandidates.push_back(
+ getGlobalDeclID(F, Record[I]));
+ break;
}
}
}
@@ -7124,6 +7130,18 @@
DynamicClasses.clear();
}
+void ASTReader::ReadUnusedLocalTypedefNameCandidates(
+ llvm::SmallSetVector<const TypedefNameDecl *, 4> &Decls) {
+ for (unsigned I = 0, N = UnusedLocalTypedefNameCandidates.size(); I != N;
+ ++I) {
+ TypedefNameDecl *D = dyn_cast_or_null<TypedefNameDecl>(
+ GetDecl(UnusedLocalTypedefNameCandidates[I]));
+ if (D)
+ Decls.insert(D);
+ }
+ UnusedLocalTypedefNameCandidates.clear();
+}
+
void
ASTReader::ReadLocallyScopedExternCDecls(SmallVectorImpl<NamedDecl *> &Decls) {
for (unsigned I = 0, N = LocallyScopedExternCDecls.size(); I != N; ++I) {
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp (revision 214046)
+++ lib/Serialization/ASTWriter.cpp (working copy)
@@ -4236,6 +4236,11 @@
}
}
+ // Build a record containing all of the UnusedLocalTypedefNameCandidates.
+ RecordData UnusedLocalTypedefNameCandidates;
+ for (const TypedefNameDecl *TD : SemaRef.UnusedLocalTypedefNameCandidates)
+ AddDeclRef(TD, UnusedLocalTypedefNameCandidates);
+
// Build a record containing all of dynamic classes declarations.
RecordData DynamicClasses;
AddLazyVectorDecls(*this, SemaRef.DynamicClasses, DynamicClasses);
@@ -4510,6 +4515,11 @@
if (!DynamicClasses.empty())
Stream.EmitRecord(DYNAMIC_CLASSES, DynamicClasses);
+ // Write the record containing potentially unused local typedefs.
+ if (!UnusedLocalTypedefNameCandidates.empty())
+ Stream.EmitRecord(UNUSED_LOCAL_TYPEDEF_NAME_CANDIDATES,
+ UnusedLocalTypedefNameCandidates);
+
// Write the record containing pending implicit instantiations.
if (!PendingInstantiations.empty())
Stream.EmitRecord(PENDING_IMPLICIT_INSTANTIATIONS, PendingInstantiations);
Index: test/Misc/ast-dump-color.cpp
===================================================================
--- test/Misc/ast-dump-color.cpp (revision 214046)
+++ test/Misc/ast-dump-color.cpp (working copy)
@@ -89,7 +89,7 @@
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[BLUE]]GuardedByAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:29[[RESET]], [[Yellow]]col:43[[RESET]]>{{$}}
//CHECK: {{^}}[[Blue]]| `-[[RESET]][[MAGENTA]]DeclRefExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:40[[RESET]]> [[Green]]'class Mutex':'class Mutex'[[RESET]][[Cyan]] lvalue[[RESET]][[Cyan]][[RESET]] [[GREEN]]Var[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]][[CYAN]] 'mu1'[[RESET]] [[Green]]'class Mutex':'class Mutex'[[RESET]]{{$}}
//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]CXXRecordDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:28:1[[RESET]], [[Yellow]]line:30:1[[RESET]]> [[Yellow]]line:28:8[[RESET]] struct[[CYAN]] Invalid[[RESET]] definition
-//CHECK: {{^}}[[Blue]]| |-[[RESET]][[GREEN]]CXXRecordDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:1[[RESET]], [[Yellow]]col:8[[RESET]]> [[Yellow]]col:8[[RESET]] implicit struct[[CYAN]] Invalid[[RESET]]
+//CHECK: {{^}}[[Blue]]| |-[[RESET]][[GREEN]]CXXRecordDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:1[[RESET]], [[Yellow]]col:8[[RESET]]> [[Yellow]]col:8[[RESET]] implicit referenced struct[[CYAN]] Invalid[[RESET]]
//CHECK: {{^}}[[Blue]]| |-[[RESET]][[GREEN]]CXXConstructorDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:29:3[[RESET]], [[Yellow]]col:42[[RESET]]> [[Yellow]]col:29[[RESET]] invalid[[CYAN]] Invalid[[RESET]] [[Green]]'void (int)'[[RESET]]
//CHECK: {{^}}[[Blue]]| | |-[[RESET]][[GREEN]]ParmVarDecl[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:37[[RESET]], [[Yellow]]<invalid sloc>[[RESET]]> [[Yellow]]col:42[[RESET]] invalid [[Green]]'int'[[RESET]]
//CHECK: {{^}}[[Blue]]| | `-[[RESET]][[BLUE]]NoInlineAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:18[[RESET]]>
Index: test/SemaCXX/implicit-exception-spec.cpp
===================================================================
--- test/SemaCXX/implicit-exception-spec.cpp (revision 214046)
+++ 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 214046)
+++ 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,192 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-local-typedefs -verify -std=c++1y -fasm-blocks %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'}}
+ using foo0alias = int ; // expected-warning {{unused type alias 'foo0alias'}}
+
+ 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; // no diag
+ typedef int 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;
+
+ // FIXME: typedef that's only used in a constexpr
+}
+
+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 <class T>
+void template_fun(T t) {
+ typedef int foo; // expected-warning {{unused typedef 'foo'}}
+ typedef int bar; // no-diag
+ bar asdf;
+
+ struct S2 {
+ typedef int Foo; // no diag
+
+ typedef int Foo2; // expected-warning {{unused typedef 'Foo2'}}
+
+ typedef int Foo3; // no diag
+ };
+
+ typename S2::Foo s2foo;
+ typename T::Foo s3foo;
+
+ typedef typename S2::Foo3 TTSF; // expected-warning {{unused typedef 'TTSF'}}
+}
+void template_fun_user() {
+ struct Local {
+ typedef int Foo; // no-diag
+ typedef int Bar; // expected-warning {{unused typedef 'Bar'}}
+ } p;
+ template_fun(p);
+}
+
+void typedef_in_nested_name() {
+ typedef struct {
+ typedef int Foo;
+ } A;
+ A::Foo adsf;
+
+ using A2 = struct {
+ typedef int Foo;
+ };
+ A2::Foo adsf2;
+}
+
+void use_in_asm() {
+ typedef struct {
+ int a;
+ int b;
+ } A;
+ __asm mov eax, [eax].A.b
+
+ using Alias = struct {
+ int a;
+ int b;
+ };
+ __asm mov eax, [eax].Alias.b
+}
+
+auto sneaky() {
+ struct S {
+ // Local typedefs can be used after the scope they were in has closed:
+ typedef int t;
+
+ // Even if they aren't, this could be an inline function that could be used
+ // in another TU, so this shouldn't warn either:
+ typedef int s;
+
+ private:
+ typedef int p; // expected-warning{{unused typedef 'p'}}
+ };
+ return S();
+}
+auto x = sneaky();
+decltype(x)::t y;
+
+static auto static_sneaky() {
+ struct S {
+ typedef int t;
+ // This function has internal linkage, so we can warn:
+ typedef int s; // expected-warning {{unused typedef 's'}}
+ };
+ return S();
+}
+auto sx = static_sneaky();
+decltype(sx)::t sy;
+
+namespace {
+auto nstatic_sneaky() {
+ struct S {
+ typedef int t;
+ // This function has internal linkage, so we can warn:
+ typedef int s; // expected-warning {{unused typedef 's'}}
+ };
+ return S();
+}
+auto nsx = nstatic_sneaky();
+decltype(nsx)::t nsy;
+}
+
+// Like sneaky(), but returning pointer to local type
+template<typename T>
+struct remove_reference { typedef T type; };
+template<typename T> struct remove_reference<T&> { typedef T type; };
+auto pointer_sneaky() {
+ struct S {
+ typedef int t;
+ typedef int s;
+ };
+ return (S*)nullptr;
+}
+remove_reference<decltype(*pointer_sneaky())>::type::t py;
+
+// Like sneaky(), but returning templated struct referencing local type.
+template <class T> struct container { int a; T t; };
+auto template_sneaky() {
+ struct S {
+ typedef int t;
+ typedef int s;
+ };
+ return container<S>();
+}
+auto tx = template_sneaky();
+decltype(tx.t)::t ty;
More information about the cfe-commits
mailing list