Introduce -Wunused-private-field. If enabled, this warning detects

unused private fields of classes that are fully defined in the current
translation unit.

llvm-svn: 158054

clang/include/clang/Basic/DiagnosticGroups.td

@@ -230,6 +230,7 @@ def UnusedComparison : DiagGroup<"unused-comparison">;
 def UnusedExceptionParameter : DiagGroup<"unused-exception-parameter">;
 def UnneededInternalDecl : DiagGroup<"unneeded-internal-declaration">;
 def UnneededMemberFunction : DiagGroup<"unneeded-member-function">;
+def UnusedPrivateField : DiagGroup<"unused-private-field">;
 def UnusedFunction : DiagGroup<"unused-function", [UnneededInternalDecl]>;
 def UnusedMemberFunction : DiagGroup<"unused-member-function",
                                      [UnneededMemberFunction]>;
@@ -311,6 +312,7 @@ def Unused : DiagGroup<"unused",
                        [UnusedArgument, UnusedFunction, UnusedLabel,
                         // UnusedParameter, (matches GCC's behavior)
                         // UnusedMemberFunction, (clean-up llvm before enabling)
+                        // UnusedPrivateField, (clean-up llvm before enabling)
                         UnusedValue, UnusedVariable]>,
                         DiagCategory<"Unused Entity Issue">;
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -173,6 +173,8 @@ def warn_unneeded_internal_decl : Warning<
 def warn_unneeded_member_function : Warning<
   "member function %0 is not needed and will not be emitted">,
   InGroup<UnneededMemberFunction>, DefaultIgnore;
+def warn_unused_private_field: Warning<"private field %0 is not used">,
+  InGroup<UnusedPrivateField>, DefaultIgnore;
 
 def warn_parameter_size: Warning<
   "%0 is a large (%1 bytes) pass-by-value argument; "

clang/include/clang/Sema/Sema.h

@@ -37,6 +37,7 @@
 #include "clang/Basic/ExpressionTraits.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/OwningPtr.h"
+#include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include <deque>
@@ -267,6 +268,11 @@ public:
   /// FieldCollector - Collects CXXFieldDecls during parsing of C++ classes.
   OwningPtr<CXXFieldCollector> FieldCollector;
 
+  typedef llvm::SmallSetVector<const NamedDecl*, 16> NamedDeclSetType;
+
+  /// \brief Set containing all declared private fields that are not used.
+  NamedDeclSetType UnusedPrivateFields;
+
   typedef llvm::SmallPtrSet<const CXXRecordDecl*, 8> RecordDeclSetTy;
 
   /// PureVirtualClassDiagSet - a set of class declarations which we have

clang/lib/Sema/Sema.cpp

@@ -30,6 +30,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
@@ -422,6 +423,79 @@ void Sema::LoadExternalWeakUndeclaredIdentifiers() {
   }
 }
 
+
+typedef llvm::DenseMap<const CXXRecordDecl*, bool> RecordCompleteMap;
+
+/// \brief Returns true, if all methods and nested classes of the given
+/// CXXRecordDecl are defined in this translation unit.
+///
+/// Should only be called from ActOnEndOfTranslationUnit so that all
+/// definitions are actually read.
+static bool MethodsAndNestedClassesComplete(const CXXRecordDecl *RD,
+                                            RecordCompleteMap &MNCComplete) {
+  RecordCompleteMap::iterator Cache = MNCComplete.find(RD);
+  if (Cache != MNCComplete.end())
+    return Cache->second;
+  if (!RD->isCompleteDefinition())
+    return false;
+  bool Complete = true;
+  for (DeclContext::decl_iterator I = RD->decls_begin(),
+                                  E = RD->decls_end();
+       I != E && Complete; ++I) {
+    if (const CXXMethodDecl *M = dyn_cast<CXXMethodDecl>(*I))
+      Complete = M->isDefined() || (M->isPure() && !isa<CXXDestructorDecl>(M));
+    else if (const CXXRecordDecl *R = dyn_cast<CXXRecordDecl>(*I)) {
+      if (R->isInjectedClassName())
+        continue;
+      if (R->hasDefinition())
+        Complete = MethodsAndNestedClassesComplete(R->getDefinition(),
+                                                   MNCComplete);
+      else
+        Complete = false;
+    }
+  }
+  MNCComplete[RD] = Complete;
+  return Complete;
+}
+
+/// \brief Returns true, if the given CXXRecordDecl is fully defined in this
+/// translation unit, i.e. all methods are defined or pure virtual and all
+/// friends, friend functions and nested classes are fully defined in this
+/// translation unit.
+///
+/// Should only be called from ActOnEndOfTranslationUnit so that all
+/// definitions are actually read.
+static bool IsRecordFullyDefined(const CXXRecordDecl *RD,
+                                 RecordCompleteMap &RecordsComplete,
+                                 RecordCompleteMap &MNCComplete) {
+  RecordCompleteMap::iterator Cache = RecordsComplete.find(RD);
+  if (Cache != RecordsComplete.end())
+    return Cache->second;
+  bool Complete = MethodsAndNestedClassesComplete(RD, MNCComplete);
+  for (CXXRecordDecl::friend_iterator I = RD->friend_begin(),
+                                      E = RD->friend_end();
+       I != E && Complete; ++I) {
+    // Check if friend classes and methods are complete.
+    if (TypeSourceInfo *TSI = (*I)->getFriendType()) {
+      // Friend classes are available as the TypeSourceInfo of the FriendDecl.
+      if (CXXRecordDecl *FriendD = TSI->getType()->getAsCXXRecordDecl())
+        Complete = MethodsAndNestedClassesComplete(FriendD, MNCComplete);
+      else
+        Complete = false;
+    } else {
+      // Friend functions are available through the NamedDecl of FriendDecl.
+      if (const FunctionDecl *FD =
+          dyn_cast<FunctionDecl>((*I)->getFriendDecl()))
+        Complete = FD->isDefined();
+      else
+        // This is a template friend, give up.
+        Complete = false;
+    }
+  }
+  RecordsComplete[RD] = Complete;
+  return Complete;
+}
+
 /// 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.
@@ -628,6 +702,23 @@ void Sema::ActOnEndOfTranslationUnit() {
     checkUndefinedInternals(*this);
   }
 
+  if (Diags.getDiagnosticLevel(diag::warn_unused_private_field,
+                               SourceLocation())
+        != DiagnosticsEngine::Ignored) {
+    RecordCompleteMap RecordsComplete;
+    RecordCompleteMap MNCComplete;
+    for (NamedDeclSetType::iterator I = UnusedPrivateFields.begin(),
+         E = UnusedPrivateFields.end(); I != E; ++I) {
+      const NamedDecl *D = *I;
+      const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D->getDeclContext());
+      if (RD && !RD->isUnion() &&
+          IsRecordFullyDefined(RD, RecordsComplete, MNCComplete)) {
+        Diag(D->getLocation(), diag::warn_unused_private_field)
+              << D->getDeclName();
+      }
+    }
+  }
+
   // Check we've noticed that we're no longer parsing the initializer for every
   // variable. If we miss cases, then at best we have a performance issue and
   // at worst a rejects-valid bug.

clang/lib/Sema/SemaDeclCXX.cpp

@@ -1439,6 +1439,17 @@ bool Sema::CheckIfOverriddenFunctionIsMarkedFinal(const CXXMethodDecl *New,
   return true;
 }
 
+static bool InitializationHasSideEffects(const FieldDecl &FD) {
+  if (!FD.getType().isNull()) {
+    if (const CXXRecordDecl *RD = FD.getType()->getAsCXXRecordDecl()) {
+      return !RD->isCompleteDefinition() ||
+             !RD->hasTrivialDefaultConstructor() ||
+             !RD->hasTrivialDestructor();
+    }
+  }
+  return false;
+}
+
 /// ActOnCXXMemberDeclarator - This is invoked when a C++ class member
 /// declarator is parsed. 'AS' is the access specifier, 'BW' specifies the
 /// bitfield width if there is one, 'InitExpr' specifies the initializer if
@@ -1624,8 +1635,23 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
 
   assert((Name || isInstField) && "No identifier for non-field ?");
 
-  if (isInstField)
-    FieldCollector->Add(cast<FieldDecl>(Member));
+  if (isInstField) {
+    FieldDecl *FD = cast<FieldDecl>(Member);
+    FieldCollector->Add(FD);
+
+    if (Diags.getDiagnosticLevel(diag::warn_unused_private_field,
+                                 FD->getLocation())
+          != DiagnosticsEngine::Ignored) {
+      // Remember all explicit private FieldDecls that have a name, no side
+      // effects and are not part of a dependent type declaration.
+      if (!FD->isImplicit() && FD->getDeclName() &&
+          FD->getAccess() == AS_private &&
+          !FD->getParent()->getTypeForDecl()->isDependentType() &&
+          !InitializationHasSideEffects(*FD))
+        UnusedPrivateFields.insert(FD);
+    }
+  }
+
   return Member;
 }
 
@@ -2105,6 +2131,25 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr *Init,
     Args = InitList->getInits();
     NumArgs = InitList->getNumInits();
   }
+
+  // Mark FieldDecl as being used if it is a non-primitive type and the
+  // initializer does not call the default constructor (which is trivial
+  // for all entries in UnusedPrivateFields).
+  // FIXME: Make this smarter once more side effect-free types can be
+  // determined.
+  if (NumArgs > 0) {
+    if (Member->getType()->isRecordType()) {
+      UnusedPrivateFields.remove(Member);
+    } else {
+      for (unsigned i = 0; i < NumArgs; ++i) {
+        if (Args[i]->HasSideEffects(Context)) {
+          UnusedPrivateFields.remove(Member);
+          break;
+        }
+      }
+    }
+  }
+
   for (unsigned i = 0; i < NumArgs; ++i) {
     SourceLocation L;
     if (InitExprContainsUninitializedFields(Args[i], Member, &L)) {
@@ -2808,6 +2853,13 @@ static bool CollectFieldInitializer(Sema &SemaRef, BaseAndFieldInfo &Info,
                                                       SourceLocation(), 0,
                                                       SourceLocation());
     Info.AllToInit.push_back(Init);
+
+    // Check whether this initializer makes the field "used".
+    Expr *InitExpr = Field->getInClassInitializer();
+    if (Field->getType()->isRecordType() ||
+        (InitExpr && InitExpr->HasSideEffects(SemaRef.Context)))
+      SemaRef.UnusedPrivateFields.remove(Field);
+
     return false;
   }
 

clang/lib/Sema/SemaExprMember.cpp

@@ -1591,6 +1591,8 @@ BuildFieldReferenceExpr(Sema &S, Expr *BaseExpr, bool IsArrow,
       MemberType = S.Context.getQualifiedType(MemberType, Combined);
   }
   
+  S.UnusedPrivateFields.remove(Field);
+
   ExprResult Base =
   S.PerformObjectMemberConversion(BaseExpr, SS.getScopeRep(),
                                   FoundDecl, Field);

clang/test/SemaCXX/warn-unused-member.cpp

@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-private-field -verify -std=c++11 %s
+
+class NotFullyDefined {
+ public:
+  NotFullyDefined();
+ private:
+  int y;
+};
+
+class HasUndefinedNestedClass {
+  class Undefined;
+  int unused_;
+};
+
+class HasUndefinedPureVirtualDestructor {
+  virtual ~HasUndefinedPureVirtualDestructor() = 0;
+  int unused_;
+};
+
+class HasDefinedNestedClasses {
+  class DefinedHere {};
+  class DefinedOutside;
+  int unused_; // expected-warning{{private field 'unused_' is not used}}
+};
+class HasDefinedNestedClasses::DefinedOutside {};
+
+class HasUndefinedFriendFunction {
+  friend void undefinedFriendFunction();
+  int unused_;
+};
+
+class HasUndefinedFriendClass {
+  friend class NotFullyDefined;
+  friend class NotDefined;
+  int unused_;
+};
+
+class HasFriend {
+  friend class FriendClass;
+  friend void friendFunction(HasFriend f);
+  int unused_; // expected-warning{{private field 'unused_' is not used}}
+  int used_by_friend_class_;
+  int used_by_friend_function_;
+};
+
+class ClassWithTemplateFriend {
+  template <typename T> friend class TemplateFriend;
+  int used_by_friend_;
+  int unused_;
+};
+
+template <typename T> class TemplateFriend {
+public:
+  TemplateFriend(ClassWithTemplateFriend my_friend) {
+    int var = my_friend.used_by_friend_;
+  }
+};
+
+class FriendClass {
+  HasFriend my_friend_;
+  void use() {
+    my_friend_.used_by_friend_class_ = 42;
+  }
+};
+
+void friendFunction(HasFriend my_friend) {
+  my_friend.used_by_friend_function_ = 42;
+}
+
+class NonTrivialConstructor {
+ public:
+  NonTrivialConstructor() {}
+};
+
+class NonTrivialDestructor {
+ public:
+  ~NonTrivialDestructor() {}
+};
+
+class Trivial {
+ public:
+  Trivial() = default;
+  Trivial(int a) {}
+};
+
+int side_effect() {
+  return 42;
+}
+
+class A {
+ public:
+  A() : primitive_type_(42), default_initializer_(), other_initializer_(42),
+        trivial_(), user_constructor_(42),
+        initialized_with_side_effect_(side_effect()) {
+    used_ = 42;
+  }
+
+  A(int x, A* a) : pointer_(a) {}
+
+ private:
+  int primitive_type_; // expected-warning{{private field 'primitive_type_' is not used}}
+  A* pointer_; // expected-warning{{private field 'pointer_' is not used}}
+  int no_initializer_; // expected-warning{{private field 'no_initializer_' is not used}}
+  int default_initializer_; // expected-warning{{private field 'default_initializer_' is not used}}
+  int other_initializer_; // expected-warning{{private field 'other_initializer_' is not used}}
+  int used_, unused_; // expected-warning{{private field 'unused_' is not used}}
+  int in_class_initializer_ = 42; // expected-warning{{private field 'in_class_initializer_' is not used}}
+  int in_class_initializer_with_side_effect_ = side_effect();
+  Trivial trivial_initializer_ = Trivial();
+  Trivial non_trivial_initializer_ = Trivial(42);
+  int initialized_with_side_effect_;
+  static int static_fields_are_ignored_;
+
+  Trivial trivial_; // expected-warning{{private field 'trivial_' is not used}}
+  Trivial user_constructor_;
+  NonTrivialConstructor non_trivial_constructor_;
+  NonTrivialDestructor non_trivial_destructor_;
+};
+
+class EverythingUsed {
+ public:
+  EverythingUsed() : as_array_index_(0), var_(by_initializer_) {
+    var_ = sizeof(sizeof_);
+    int *use = &by_reference_;
+    int test[2];
+    test[as_array_index_] = 42;
+  }
+
+  template<class T>
+  void useStuff(T t) {
+    by_template_function_ = 42;
+  }
+
+ private:
+  int var_;
+  int sizeof_;
+  int by_reference_;
+  int by_template_function_;
+  int as_array_index_;
+  int by_initializer_;
+};
+
+namespace mutual_friends {
+// Undefined methods make mutual friends undefined.
+class A {
+  int a;
+  friend class B;
+  void doSomethingToAOrB();
+};
+class B {
+  int b;
+  friend class A;
+};
+
+// Undefined friends do not make a mutual friend undefined.
+class C {
+  int c;
+  void doSomethingElse() {}
+  friend class E;
+  friend class D;
+};
+class D {
+  int d; // expected-warning{{private field 'd' is not used}}
+  friend class C;
+};
+
+// Undefined nested classes make mutual friends undefined.
+class F {
+  int f;
+  class G;
+  friend class H;
+};
+class H {
+  int h;
+  friend class F;
+};
+}  // namespace mutual_friends
+
+namespace anonymous_structs_unions {
+class A {
+ private:
+  // FIXME: Look at the DeclContext for anonymous structs/unions.
+  union {
+    int *Aligner;
+    unsigned char Data[8];
+  };
+};
+union S {
+ private:
+  int *Aligner;
+  unsigned char Data[8];
+};
+}  // namespace anonymous_structs_unions