Handle dependencies on 'using' declarations properly. If we

use a type via a using declaration, we need to make sure to
use that using declaration as well.  It may be in a different
file from the type!

There may be corner cases we don't handle right yet, but we do
the right thing at least most of the time.  One case we don't
handle is when there's a using declaration but we ignore it
('using foo::a; foo::a = 1').  We'd do better if clang would
report whether a given decl-use was via a using declaration or
not, but right now it doesn't.  cf:
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2011-April/014564.html
(though that applies only to using vars, not functions).

In order to implement this reasonably, I had to (finally!)
unify the type and decl reporting in iwyu.cc, rather than
iwyu_output.cc.  This allows me to remove a bit of duplicated
code, and also makes the logic easier to follow.

R=dsturtevant
DELTA=358  (286 added, 71 deleted, 1 changed)


Revision created by MOE tool push_codebase.
MOE_MIGRATION=1576

iwyu.cc

@@ -1190,6 +1190,16 @@ struct VisitorState {
   // instantiated calls, we can't store the exprs themselves, but have
   // to store their location.)
   set<SourceLocation> processed_overload_locs;
+
+  // When we see a using declaration, we want to keep track of what
+  // file it's in, because other files may depend on that using
+  // declaration to get the names of their types right.  We want to
+  // make sure we don't replace an #include with a forward-declare
+  // when we might need the #include's using declaration.
+  // The key is the type being 'used', the FileEntry is the file
+  // that has the using decl.  If there are multiple using decls
+  // for a file, we prefer the one that has NamedDecl in it.
+  multimap<const NamedDecl*, const UsingDecl*> using_declarations;
 };
 
 
@@ -1357,61 +1367,34 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
       return GetInstantiationLoc(use_loc);
     return use_loc;
   }
-  SourceLocation GetUseLocationForMacroExpansion(SourceLocation use_loc,
-                                                 const Type* used_type) {
-    used_type = RemovePointersAndReferencesAsWritten(used_type);
-    if (const Decl* decl = TypeToDeclAsWritten(used_type))
-      return GetUseLocationForMacroExpansion(use_loc, decl);
-    return use_loc;
-  }
 
   //------------------------------------------------------------
   // Checkers, that tell iwyu_output about uses of symbols.
   // We let, but don't require, subclasses to override these.
 
-  // Called when the given type is fully used at used_loc, regardless
-  // of the type being explicitly written in the source code or not.
-  virtual void ReportTypeUse(SourceLocation used_loc, const Type* type) {
-    // TODO(csilvers): figure out if/when calling CanIgnoreType() is correct.
-
-    // Map private types like __normal_iterator to their public counterpart.
-    type = MapPrivateTypeToPublicType(type);
-    // Figure out the best location to attribute uses inside macros.
-    if (IsInMacro(used_loc))
-      used_loc = GetUseLocationForMacroExpansion(used_loc, type);
-    const FileEntry* used_in = GetFileEntry(used_loc);
-    preprocessor_info().FileInfoFor(used_in)->ReportFullSymbolUse(
-        used_loc, type, IsNodeInsideCXXMethodBody(current_ast_node()));
-  }
-
-  virtual void ReportTypesUse(SourceLocation used_loc,
-                              const set<const Type*>& types) {
-    for (Each<const Type*> it(&types); !it.AtEnd(); ++it)
-      ReportTypeUse(used_loc, *it);
-  }
-
-  // Called when a type is used in a forward-declare context.
-  virtual void ReportTypeForwardDeclareUse(SourceLocation used_loc,
-                                           const Type* type) {
-    // TODO(csilvers): figure out if/when calling CanIgnoreType() is correct.
-    type = MapPrivateTypeToPublicType(type);
-    // Figure out the best location to attribute uses inside macros.
-    if (IsInMacro(used_loc))
-      used_loc = GetUseLocationForMacroExpansion(used_loc, type);
-    const FileEntry* used_in = GetFileEntry(used_loc);
-    preprocessor_info().FileInfoFor(used_in)->ReportForwardDeclareUse(
-        used_loc, type, IsNodeInsideCXXMethodBody(current_ast_node()));
-  }
-
   virtual void ReportDeclUse(SourceLocation used_loc, const NamedDecl* decl) {
     // Map private decls like __normal_iterator to their public counterpart.
     decl = MapPrivateDeclToPublicDecl(decl);
     if (CanIgnoreDecl(decl))
       return;
+
     // Figure out the best location to attribute uses inside macros.
     if (IsInMacro(used_loc))
       used_loc = GetUseLocationForMacroExpansion(used_loc, decl);
     const FileEntry* used_in = GetFileEntry(used_loc);
+
+    // If we're a use that depends on a using declaration, make sure
+    // we #include the file with the using declaration.
+    // TODO(csilvers): check that our getQualifier() does not match
+    // the namespace of the decl.  If we have 'using std::vector;' +
+    // 'std::vector<int> foo;' we don't actually care about the
+    // using-decl.
+    if (const UsingDecl* using_decl
+        = GetUsingDeclarationOf(decl, GetDeclContext(current_ast_node()))) {
+      preprocessor_info().FileInfoFor(used_in)->ReportFullSymbolUse(
+          used_loc, using_decl, IsNodeInsideCXXMethodBody(current_ast_node()));
+    }
+
     preprocessor_info().FileInfoFor(used_in)->ReportFullSymbolUse(
         used_loc, decl, IsNodeInsideCXXMethodBody(current_ast_node()));
   }
@@ -1427,14 +1410,57 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     decl = MapPrivateDeclToPublicDecl(decl);
     if (CanIgnoreDecl(decl))
       return;
+
     // Figure out the best location to attribute uses inside macros.
     if (IsInMacro(used_loc))
       used_loc = GetUseLocationForMacroExpansion(used_loc, decl);
     const FileEntry* used_in = GetFileEntry(used_loc);
+
+    // If we're a use that depends on a using declaration, make sure
+    // we #include the file with the using declaration.
+    if (const UsingDecl* using_decl
+        = GetUsingDeclarationOf(decl, GetDeclContext(current_ast_node()))) {
+      preprocessor_info().FileInfoFor(used_in)->ReportFullSymbolUse(
+          used_loc, using_decl, IsNodeInsideCXXMethodBody(current_ast_node()));
+    }
+
     preprocessor_info().FileInfoFor(used_in)->ReportForwardDeclareUse(
         used_loc, decl, IsNodeInsideCXXMethodBody(current_ast_node()));
   }
 
+  // Called when the given type is fully used at used_loc, regardless
+  // of the type being explicitly written in the source code or not.
+  virtual void ReportTypeUse(SourceLocation used_loc, const Type* type) {
+    // TODO(csilvers): figure out if/when calling CanIgnoreType() is correct.
+    if (!type)
+      return;
+
+    // Map private types like __normal_iterator to their public counterpart.
+    type = MapPrivateTypeToPublicType(type);
+    // For the below, we want to be careful to call *our*
+    // ReportDeclUse(), not any of the ones in subclasses.
+    if (IsPointerOrReferenceAsWritten(type)) {
+      type = RemovePointersAndReferencesAsWritten(type);
+      if (const NamedDecl* decl = TypeToDeclAsWritten(type)) {
+        VERRS(6) << "(For pointer type " << PrintableType(type) << "):\n";
+        IwyuBaseAstVisitor<Derived>::ReportDeclForwardDeclareUse(used_loc,
+                                                                 decl);
+      }
+    } else {
+      if (const NamedDecl* decl = TypeToDeclAsWritten(type)) {
+        decl = GetDefinitionAsWritten(decl);
+        VERRS(6) << "(For type " << PrintableType(type) << "):\n";
+        IwyuBaseAstVisitor<Derived>::ReportDeclUse(used_loc, decl);
+      }
+    }
+  }
+
+  virtual void ReportTypesUse(SourceLocation used_loc,
+                              const set<const Type*>& types) {
+    for (Each<const Type*> it(&types); !it.AtEnd(); ++it)
+      ReportTypeUse(used_loc, *it);
+  }
+
   //------------------------------------------------------------
   // Visitors of types derived from clang::Decl.
 
@@ -2125,6 +2151,12 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     return visitor_state_->preprocessor_info;
   }
 
+  void AddUsingDeclaration(const NamedDecl* target_decl,  // what's being used
+                           const UsingDecl* using_decl) {
+    visitor_state_->using_declarations.insert(make_pair(target_decl,
+                                                        using_decl));
+  }
+
  private:
   template <typename T> friend class IwyuBaseAstVisitor;
 
@@ -2136,6 +2168,27 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     visitor_state_->processed_overload_locs.insert(loc);
   }
 
+  const UsingDecl* GetUsingDeclarationOf(const NamedDecl* decl,
+                                         const DeclContext* using_context) {
+    // We look through all the using-decls of the given decl.  We
+    // limit them to ones that are visible from the decl-context we're
+    // currently in (that is, what namespaces we're in).  Of those, we
+    // pick the one that's in the same file as decl, if possible,
+    // otherwise we pick one arbitrarily.
+    const UsingDecl* retval = NULL;
+    vector<const UsingDecl*> using_decls
+        = FindInMultiMap(visitor_state_->using_declarations, decl);
+    for (Each<const UsingDecl*> it(&using_decls); !it.AtEnd(); ++it) {
+      if (!(*it)->getDeclContext()->Encloses(using_context))
+        continue;
+      if (GetFileEntry(decl) == GetFileEntry(*it) ||    // in same file, prefer
+          retval == NULL) {        // not in same file, but better than nothing
+        retval = *it;
+      }
+    }
+    return retval;
+  }
+
   // Do not any variables here!  If you do, they will not be shared
   // between the normal iwyu ast visitor and the
   // template-instantiation visitor, which is almost always a mistake.
@@ -2926,7 +2979,19 @@ class IwyuAstConsumer
   }
 
   bool VisitUsingDecl(clang::UsingDecl* decl) {
+    // If somebody in a different file tries to use one of these decls
+    // with the shortened name, then they had better #include us in
+    // order to get our using declaration.  We store the necessary
+    // information here.  Note: we have to store this even if this is
+    // an ast node we would otherwise ignore, since other AST nodes
+    // (which we might not ignore) can depend on it.
+    for (UsingDecl::shadow_iterator it = decl->shadow_begin();
+         it != decl->shadow_end(); ++it) {
+      AddUsingDeclaration((*it)->getTargetDecl(), decl);
+    }
+
     if (CanIgnoreCurrentASTNode())  return true;
+
     // The shadow decls hold the declarations for the var/fn/etc we're
     // using.  (There may be more than one if, say, we're using an
     // overloaded function.)  We check to make sure nothing we're
@@ -2935,6 +3000,7 @@ class IwyuAstConsumer
          it != decl->shadow_end(); ++it) {
       ReportDeclForwardDeclareUse(CurrentLoc(), (*it)->getTargetDecl());
     }
+
     return Base::VisitUsingDecl(decl);
   }
 

iwyu_ast_util.cc

@@ -49,6 +49,7 @@ using clang::ClassTemplateDecl;
 using clang::ClassTemplatePartialSpecializationDecl;
 using clang::ClassTemplateSpecializationDecl;
 using clang::Decl;
+using clang::DeclContext;
 using clang::DeclRefExpr;
 using clang::DeclaratorDecl;
 using clang::DependentNameType;
@@ -320,6 +321,14 @@ bool IsMemberOfATypedef(const ASTNode* ast_node) {
   return false;
 }
 
+const DeclContext* GetDeclContext(const ASTNode* ast_node) {
+  for (; ast_node != NULL; ast_node = ast_node->parent()) {
+    if (ast_node->IsA<Decl>())
+      return ast_node->GetAs<Decl>()->getDeclContext();
+  }
+  return NULL;
+}
+
 
 //------------------------------------------------------------
 // Helper functions for working with raw Clang AST nodes.

iwyu_ast_util.h

@@ -415,6 +415,9 @@ const clang::NestedNameSpecifier* GetQualifier(const ASTNode* ast_node);
 // *not* return true if the ast_node itself is a typedef.
 bool IsMemberOfATypedef(const ASTNode* ast_node);
 
+// Returns the decl-context of the deepest decl in the ast-chain.
+const clang::DeclContext* GetDeclContext(const ASTNode* ast_node);
+
 
 //------------------------------------------------------------
 // Helper functions for working with raw Clang AST nodes.

iwyu_output.cc

@@ -488,33 +488,6 @@ void IwyuFileInfo::ReportFullSymbolUse(SourceLocation use_loc,
   }
 }
 
-// Converts the type into a decl, and reports that.
-void IwyuFileInfo::ReportFullSymbolUse(SourceLocation use_loc,
-                                       const Type* type,
-                                       bool in_cxx_method_body) {
-  if (!type)
-    return;
-  // The 'full info' for a pointer type can be satisfied by
-  // forward-declaring its underlying type.
-  if (IsPointerOrReferenceAsWritten(type)) {
-    type = RemovePointersAndReferencesAsWritten(type);
-    const NamedDecl* decl = TypeToDeclAsWritten(type);
-    if (decl) {
-      symbol_uses_.push_back(OneUse(decl, use_loc, OneUse::kForwardDeclareUse,
-                                    in_cxx_method_body));
-      LogSymbolUse("Marked (fwd decl) use of pointer to", symbol_uses_.back());
-    }
-  } else {
-    const NamedDecl* decl = TypeToDeclAsWritten(type);
-    if (decl) {
-      decl = GetDefinitionAsWritten(decl);
-      symbol_uses_.push_back(OneUse(decl, use_loc, OneUse::kFullUse,
-                                    in_cxx_method_body));
-      LogSymbolUse("Marked full-info use of type", symbol_uses_.back());
-    }
-  }
-}
-
 void IwyuFileInfo::ReportFullSymbolUse(SourceLocation use_loc,
                                        const string& dfn_filepath,
                                        const string& symbol) {

iwyu_output.h

@@ -225,9 +225,6 @@ class IwyuFileInfo {
   void ReportFullSymbolUse(clang::SourceLocation use_loc,
                            const clang::NamedDecl* decl,
                            bool in_cxx_method_body);
-  void ReportFullSymbolUse(clang::SourceLocation use_loc,
-                           const clang::Type* type,
-                           bool in_cxx_method_body);
   // This will mostly be used for macro tokens.
   void ReportFullSymbolUse(clang::SourceLocation use_loc,
                            const string& dfn_filepath,

tests/include_with_using-d1.h

@@ -0,0 +1,14 @@
+//===--- include_with_using-d1.h - test input file for iwyu ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+namespace ns {
+class PtrInNs {};
+}
+
+using ns::PtrInNs;

tests/include_with_using-d2.h

@@ -0,0 +1,21 @@
+//===--- include_with_using-d1.h - test input file for iwyu ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// This class isn't actually used by the .cc file.  The purpose of
+// this code is to make sure that the .cc file doesn't see the need to
+// #include -d2.h just because of the using declaration (the using
+// declaration should actually be needed by the .cc file for that to
+// happen).
+namespace ns2 {
+class PtrInNs2 {};
+}
+
+using ns2::PtrInNs2;
+
+class UsedFromD2 { };

tests/include_with_using-d3.h

@@ -0,0 +1,13 @@
+//===--- include_with_using-d3.h - test input file for iwyu ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+namespace ns3 {
+class PtrInNs3 {};
+}
+// The using decl for PtrInNs3 is in include_with_using-d3b.h

tests/include_with_using-d3b.h

@@ -0,0 +1,10 @@
+//===--- include_with_using-d3b.h - test input file for iwyu --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+using ns3::PtrInNs3;

tests/include_with_using-d4.h

@@ -0,0 +1,16 @@
+//===--- include_with_using-d4.h - test input file for iwyu ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Also test using a var, not a type;
+
+namespace ns4 {
+int var_in_d4 = 0;
+}
+
+using ns4::var_in_d4;

tests/include_with_using-d5.h

@@ -0,0 +1,16 @@
+//===--- include_with_using-d5.h - test input file for iwyu ---------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+namespace ns5 {
+class PtrInNs5 {};
+}
+// This using decl won't be used because it's not in the right namespace
+namespace ns_unused {
+using ns5::PtrInNs5;
+}

tests/include_with_using-d5b.h

@@ -0,0 +1,13 @@
+//===--- include_with_using-d5b.h - test input file for iwyu --------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// This using decl *will* be used because it's in the right namespace
+namespace ns_cc {
+using ns5::PtrInNs5;
+}

tests/include_with_using.cc

@@ -0,0 +1,64 @@
+//===--- include_with_using.cc - test input file for iwyu -----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// Tests that if we use a symbol from a .h file that the .h file has
+// a 'using' declaration for, we don't consider replacing the use with
+// a forward-declaration.  On the other hand, if we don't depend on
+// the using declaration, replacing is fair game.
+
+// TODO(csilvers): also test UsingDirectiveDecl ('using namespace std').
+
+#include "tests/include_with_using-d1.h"
+#include "tests/include_with_using-d2.h"
+#include "tests/include_with_using-d3.h"
+#include "tests/include_with_using-d3b.h"
+#include "tests/include_with_using-d4.h"
+#include "tests/include_with_using-d5.h"
+#include "tests/include_with_using-d5b.h"
+
+PtrInNs* pin = 0;
+
+UsedFromD2* ufd2 = 0;
+
+// Needs the using decl from d3b (but not the type decl from d3!).
+PtrInNs3* pin3 = 0;
+
+namespace ns_cc {     // thus, we need the using-decl from d5b.h, not d5.h
+namespace internal {
+PtrInNs5* pin5 = 0;
+}
+}
+
+int main() {
+  // Needs the using decl from d4.
+  var_in_d4 = 0;
+}
+
+/**** IWYU_SUMMARY
+
+tests/include_with_using.cc should add these lines:
+class UsedFromD2;
+namespace ns3 { class PtrInNs3; }
+namespace ns5 { class PtrInNs5; }
+
+tests/include_with_using.cc should remove these lines:
+- #include "tests/include_with_using-d2.h"  // lines XX-XX
+- #include "tests/include_with_using-d3.h"  // lines XX-XX
+- #include "tests/include_with_using-d5.h"  // lines XX-XX
+
+The full include-list for tests/include_with_using.cc:
+#include "tests/include_with_using-d1.h"  // for PtrInNs
+#include "tests/include_with_using-d3b.h"  // for PtrInNs3
+#include "tests/include_with_using-d4.h"  // for var_in_d4
+#include "tests/include_with_using-d5b.h"  // for PtrInNs5
+class UsedFromD2;
+namespace ns3 { class PtrInNs3; }
+namespace ns5 { class PtrInNs5; }
+
+***** IWYU_SUMMARY */