[patch] Implementing -Wunused-local-typedefs

Nico Weber thakis at chromium.org
Fri Sep 5 16:44:10 PDT 2014


On Fri, Sep 5, 2014 at 2:44 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> Thanks, this is looking really good.
>
> +  if (Diags.isIgnored(diag::warn_unused_local_typedef, SourceLocation()))
> +    return;
>
> It would seem better to consider the diagnostic state at the point where
> the typedef is declared. (Maybe check this when adding typedefs to the set
> rather than when diagnosing?)
>

Great catch. -Wunused-private-fields probably gets this wrong too. I added
a test for this (see the very bottom of warn-unused-local-typedef.cpp, and
the pragma diagnostic push/pop). The test sadly showed that adding when
adding the typedefs is wrong too: For template instantiations, the
instantiation is done at end-of-file, but a pragma diag at end of file
shouldn't disable warnings in templates that physically are further up. So
I just removed this check.


> +    // Warnigns emitted in ActOnEndOfTranslationUnit() should be emitted
> for
>
> Typo 'Warnigns'.
>

Doen.


> +  // 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());
>
> Why are dependent local classes skipped here? I'd like at least a comment
> to explain this so future readers aren't surprised :)
>

I added a short comment. This is covered by my tests, so removing it does
make something fail.
(SemaTemplateInstantiateDecl.cpp's Sema::BuildVariableInstantiation() calls
this method for instantiated fields, so the warnings are deferred until
after instantiation.)


>
> +  for (auto *TmpD : D->decls()) {
> +    if (const auto* T = dyn_cast<TypedefNameDecl>(TmpD))
> +      DiagnoseUnusedDecl(T);
> +    else if(const auto *R = dyn_cast<RecordDecl>(TmpD))
>
> '*' on the right, please.
>


Done.


>
> +  if (auto* TD = dyn_cast<TypedefNameDecl>(D)) {
> +    // typedefs can be referenced later on, so the diagnostics are emitted
> +    // at end-of-translation-unit.
>
> Likewise.
>


Done.


>
> +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);
> +  }
> +}
> [...]
> -    if (!S->hasUnrecoverableErrorOccurred())
> +    if (!S->hasUnrecoverableErrorOccurred()) {
>        DiagnoseUnusedDecl(D);
> +      if (const auto *RD = dyn_cast<RecordDecl>(D))
> +        DiagnoseUnusedNestedTypedefs(RD);
> +    }
>
> Would it make sense for DiagnoseUnusedDecl to do the recursive walk over
> child decls itself?
>

Doesn't matter too much either way I guess? Left it as is for now.


>
> +/// In a function like
> +/// auto f() {
> +///   struct S { typedef int a; };
> +///   return S;
> +/// }
>
> Should be 'return S();' or similar here.
>

Done.


> +/// others. Pretend that all local typedefs are always references, to not
> warn
>
> Typo 'references'.
>

Done.


> +class LocalTypedefNameReferencer
> +    : public RecursiveASTVisitor<LocalTypedefNameReferencer> {
> +public:
> +  LocalTypedefNameReferencer(Sema &S) : S(S) {}
> +  bool VisitRecordType(const RecordType *RT);
> +private:
> +  Sema &S;
> +};
>
> I think you'll also need to handle MemberPointerType here; the `Class` in
> `T Class::*` is just modeled as a CXXRecordDecl, not a RecordType.
>

I added the (slightly contrived :-) ) test you showed me, it seems to pass
as-is.


> +      if (T->getAccess() != AS_private)
>
> You should also check if the class has any friends; if so, they might use
> the member.
>

Good point. Done, added a test for this.


> +  }
>    else if (TypeDecl *TD = dyn_cast<TypeDecl>(FoundDecl))
>
> No newline between } and else.
>

Done.


> +  // Build a record containing all of the
> UnusedLocalTypedefNameCandidates.
> +  RecordData UnusedLocalTypedefNameCandidates;
> +  for (const TypedefNameDecl *TD :
> SemaRef.UnusedLocalTypedefNameCandidates)
> +    AddDeclRef(TD, UnusedLocalTypedefNameCandidates);
>
> Maybe wrap this in a `if (!isModule)`
>

UnusedLocalTypedefNameCandidates should've been clear()ed for modules at
this point I think, so should be fine as is.


> +  // FIXME: typedef that's only used in a constexpr
>
> Do you mean "in a constant expression"? constexpr is an adjective, not a
> noun.
>

Done (by fixing the FIXME).

Thanks for the thorough review!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140905/e68dbdbf/attachment.html>
-------------- next part --------------
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td	(revision 217285)
+++ include/clang/Basic/DiagnosticGroups.td	(working copy)
@@ -402,6 +402,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">;
@@ -526,7 +527,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">;
 
@@ -622,6 +623,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 217285)
+++ 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 217285)
+++ 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 217285)
+++ 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 217285)
+++ include/clang/Sema/Sema.h	(working copy)
@@ -389,6 +389,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
@@ -1048,6 +1052,8 @@
   /// \brief Retrieve the module loader associated with the preprocessor.
   ModuleLoader &getModuleLoader() const;
 
+  void emitAndClearUnusedLocalTypedefWarnings();
+
   void ActOnEndOfTranslationUnit();
 
   void CheckDelegatingCtorCycles();
@@ -3209,6 +3215,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 217285)
+++ 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 217285)
+++ include/clang/Serialization/ASTReader.h	(working copy)
@@ -759,6 +759,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.
@@ -1789,6 +1794,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 217285)
+++ 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 217285)
+++ lib/Sema/Sema.cpp	(working copy)
@@ -597,6 +597,19 @@
   return Complete;
 }
 
+void Sema::emitAndClearUnusedLocalTypedefWarnings() {
+  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();
+  }
+  UnusedLocalTypedefNameCandidates.clear();
+}
+
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
@@ -719,6 +732,10 @@
       }
     }
 
+    // Warnings emitted in ActOnEndOfTranslationUnit() should be emitted for
+    // modules when they are built, not every time they are used.
+    emitAndClearUnusedLocalTypedefWarnings();
+
     // Modules don't need any of the checking below.
     TUScope = nullptr;
     return;
@@ -827,6 +844,8 @@
     if (ExternalSource)
       ExternalSource->ReadUndefinedButUsed(UndefinedButUsed);
     checkUndefinedButUsed(*this);
+
+    emitAndClearUnusedLocalTypedefWarnings();
   }
 
   if (!Diags.isIgnored(diag::warn_unused_private_field, SourceLocation())) {
Index: lib/Sema/SemaCXXScopeSpec.cpp
===================================================================
--- lib/Sema/SemaCXXScopeSpec.cpp	(revision 217285)
+++ 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 217285)
+++ lib/Sema/SemaDecl.cpp	(working copy)
@@ -380,6 +380,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
@@ -928,6 +929,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);
@@ -1395,10 +1397,22 @@
 
   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()))
+    // For dependent types, the diagnostic is deferred.
+    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.
@@ -1461,11 +1475,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);
@@ -1505,8 +1538,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 217285)
+++ 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"
@@ -2710,6 +2711,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 referenced, 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 || R->hasFriends())
+        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,
@@ -2755,6 +2790,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 217285)
+++ lib/Sema/SemaStmtAsm.cpp	(working copy)
@@ -467,9 +467,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))
+  } else if (TypeDecl *TD = dyn_cast<TypeDecl>(FoundDecl))
     RT = TD->getTypeForDecl()->getAs<RecordType>();
   if (!RT)
     return true;
Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp	(revision 217285)
+++ lib/Sema/SemaTemplate.cpp	(working copy)
@@ -7945,6 +7945,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 217285)
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp	(working copy)
@@ -1200,6 +1200,9 @@
     SemaRef.InstantiateClassMembers(D->getLocation(), Record, TemplateArgs,
                                     TSK_ImplicitInstantiation);
   }
+
+  SemaRef.DiagnoseUnusedNestedTypedefs(Record);
+
   return Record;
 }
 
@@ -3653,7 +3656,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 217285)
+++ lib/Serialization/ASTReader.cpp	(working copy)
@@ -3285,6 +3285,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;
     }
   }
 }
@@ -7196,6 +7202,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 217285)
+++ lib/Serialization/ASTWriter.cpp	(working copy)
@@ -4284,6 +4284,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);
@@ -4561,6 +4566,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 217285)
+++ 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/Modules/Inputs/module.map
===================================================================
--- test/Modules/Inputs/module.map	(revision 217285)
+++ test/Modules/Inputs/module.map	(working copy)
@@ -292,6 +292,10 @@
   header "warning.h"
 }
 
+module warn_unused_local_typedef {
+  header "warn-unused-local-typedef.h"
+}
+
 module initializer_list {
   header "initializer_list"
 }
Index: test/Modules/Inputs/warn-unused-local-typedef.h
===================================================================
--- test/Modules/Inputs/warn-unused-local-typedef.h	(revision 0)
+++ test/Modules/Inputs/warn-unused-local-typedef.h	(working copy)
@@ -0,0 +1 @@
+inline void myfun() { typedef int a; }
Index: test/Modules/warn-unused-local-typedef.cpp
===================================================================
--- test/Modules/warn-unused-local-typedef.cpp	(revision 0)
+++ test/Modules/warn-unused-local-typedef.cpp	(working copy)
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang -Wunused-local-typedef -c -x objective-c++ -fcxx-modules -fmodules -fmodules-cache-path=%t -I %S/Inputs %s 2>&1 | FileCheck %s -check-prefix=CHECK_1
+// RUN: %clang -Wunused-local-typedef -c -x objective-c++ -fcxx-modules -fmodules -fmodules-cache-path=%t -I %S/Inputs %s 2>&1 | FileCheck %s -check-prefix=CHECK_2 -allow-empty
+
+// For modules, the warning should only fire the first time, when the module is
+// built.
+// CHECK_1: warning: unused typedef
+// CHECK_2-NOT: warning: unused typedef
+ at import warn_unused_local_typedef;
Index: test/SemaCXX/implicit-exception-spec.cpp
===================================================================
--- test/SemaCXX/implicit-exception-spec.cpp	(revision 217285)
+++ 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 217285)
+++ 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-typedef-serialize.cpp
===================================================================
--- test/SemaCXX/warn-unused-local-typedef-serialize.cpp	(revision 0)
+++ test/SemaCXX/warn-unused-local-typedef-serialize.cpp	(working copy)
@@ -0,0 +1,11 @@
+// RUN: %clang -x c++-header -c -Wunused-local-typedef %s -o %t.gch -Werror
+// RUN: %clang -DBE_THE_SOURCE -c -Wunused-local-typedef -include %t %s 2>&1 | FileCheck %s
+// RUN: %clang -DBE_THE_SOURCE -c -Wunused-local-typedef -include %t %s 2>&1 | FileCheck %s
+
+#ifndef BE_THE_SOURCE
+inline void myfun() {
+// The warning should fire every time the pch file is used, not when it's built.
+// CHECK: warning: unused typedef
+  typedef int a;
+}
+#endif
Index: test/SemaCXX/warn-unused-local-typedef.cpp
===================================================================
--- test/SemaCXX/warn-unused-local-typedef.cpp	(revision 0)
+++ test/SemaCXX/warn-unused-local-typedef.cpp	(working copy)
@@ -0,0 +1,232 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-local-typedef -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;
+
+  typedef int ConstExprInt;
+  static constexpr int a = (ConstExprInt)4;
+}
+
+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));
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused-local-typedef"
+  typedef int trungl_bot_was_here; // no diag
+#pragma clang diagnostic pop
+
+  typedef int foo; // expected-warning {{unused typedef 'foo'}}
+}
+
+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
+
+    // XXX this doesn't warn :-/
+    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;
+
+auto sneaky_with_friends() {
+  struct S {
+  private:
+    friend class G;
+    // Can't warn if we have friends:
+    typedef int p;
+  };
+  return S();
+}
+
+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;
+
+// Like sneaky(), but doing its sneakiness by returning a member function
+// pointer.
+auto sneaky_memfun() {
+  struct S {
+    typedef int type;
+    int n;
+  };
+  return &S::n;
+}
+
+template <class T> void sneaky_memfun_g(int T::*p) {
+  typename T::type X;
+}
+
+void sneaky_memfun_h() {
+  sneaky_memfun_g(sneaky_memfun());
+}
+
+// This should not disable any warnings:
+#pragma clang diagnostic ignored "-Wunused-local-typedef"


More information about the cfe-commits mailing list