Thanks, comments inline... This is sadly a pretty cursory read, sorry, working on other stuff. I think, especially given the "used" concept at play here, Richard or Eli looking at this would be very good. =] Thanks again for working on it.<div>
<br></div><div><div>--- a/include/clang/Basic/DiagnosticSemaKinds.td</div><div>+++ b/include/clang/Basic/DiagnosticSemaKinds.td</div><div>@@ -134,6 +134,9 @@  def warn_unneeded_member_function : Warning<</div><div>   "member function %0 is not needed and will not be emitted">,</div>
<div>   InGroup<UnneededMemberFunction>, DefaultIgnore;</div><div> </div><div>+def warn_unused_private_field: Warning<"private field %0 is not used">,</div><div>+  InGroup<UnusedPrivateField>, DefaultIgnore;</div>
<div>+</div></div><div><br></div><div>I wouldn't separate this in its own group w/ vertical whitespace, but rather jam it in with the other unused warning messages.</div><div><br></div><div><div>--- a/lib/Sema/SemaDeclCXX.cpp</div>
<div>+++ b/lib/Sema/SemaDeclCXX.cpp</div><div>@@ -1617,8 +1617,25 @@  Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,</div><div> </div><div>   assert((Name || isInstField) && "No identifier for non-field ?");</div>
<div> </div><div>-  if (isInstField)</div><div>-    FieldCollector->Add(cast<FieldDecl>(Member));</div><div>+  if (isInstField) {</div><div>+    FieldDecl *FD = cast<FieldDecl>(Member);</div><div>+    FieldCollector->Add(FD);</div>
<div>+    if (FD->getAccess() == AS_private) {</div><div>+      bool hasNoSideEffects = true;</div><div>+      if (!FD->getType().isNull()) {</div><div>+        if (const CXXRecordDecl *RD =</div><div>+            FD->getType().getTypePtrOrNull()->getAsCXXRecordDecl()) {</div>
</div><div><br></div><div>You shouldn't need getTypePtrOrNull() here. QualType provides an -> overload that delegates to Type.</div><div><br></div><div><div>+      if (!FD->getParent()->getTypeForDecl()->isInstantiationDependentType() &&</div>
<div>+          hasNoSideEffects)</div></div><div><br></div><div>I'm a bit surprised at checking InstantiationDependentType here... Not just Dependent... Also some comments as to what this is supposed to handle would be good...</div>
<div><br></div><div><div>@@ -2063,6 +2080,15 @@  Sema::BuildMemberInitializer(ValueDecl *Member,</div><div>   assert((DirectMember || IndirectMember) &&</div><div>          "Member must be a FieldDecl or IndirectFieldDecl");</div>
<div> </div><div>+  // FIXME: Allow more initializers not to count as usage, e.g. where all</div><div>+  // arguments are primitive types.</div><div>+  ValueDecl *VD = dyn_cast<ValueDecl>(Member);</div><div><br></div>
<div>If you know the cast will always succeed, just use "cast" rather than "dyn_cast".</div><div><br></div><div>+  if (!VD->getType()->isBuiltinType() &&</div><div>+      !VD->getType()->isAnyPointerType() &&</div>
</div><div><br></div><div>Why are these here? We already have constraints on the type of things above when we check the Field Decl? Maybe I just don't understand what these are doing.</div><div><br></div><div><div>--- a/lib/Sema/SemaTemplateInstantiate.cpp</div>
<div>+++ b/lib/Sema/SemaTemplateInstantiate.cpp</div><div>@@ -1791,6 +1791,19 @@  Sema::InstantiateClass(SourceLocation PointOfInstantiation,</div><div>         if (OldField->getInClassInitializer())</div><div>           FieldsWithMemberInitializers.push_back(std::make_pair(OldField,</div>
<div>                                                                 Field));</div><div>+        if (Field->getAccess() == AS_private) {</div><div>+          bool hasNoSideEffects = true;</div><div>+          if (!Field->getType().isNull()) {</div>
<div>+            if (const CXXRecordDecl *RD =</div><div>+                Field->getType().getTypePtrOrNull()->getAsCXXRecordDecl()) {</div><div>+              hasNoSideEffects = RD->isCompleteDefinition() &&</div>
<div>+                                 RD->hasTrivialDefaultConstructor() &&</div><div>+                                 RD->hasTrivialDestructor();</div><div>+            }</div><div>+          }</div><div>
+          if (hasNoSideEffects)</div><div>+            UnusedPrivateFields.insert(Field);</div><div>+        }</div></div><div><br></div><div>This is the second copy of this pattern. We should either have template instantiation call the not-template Sema routines to do this, or we should factor the predicate into a helper somewhere, likely FieldDecl.</div>