Ignore the class qualifier in out of line methods

Instead of aborting the AST traversal completely whenever a class
qualifier is found, skip the traversal of the class template and its
methods in such contexts only.

The reasoning here is that a class template will be inspected during the
regular instantiation anyway, so there is no need to duplicate efforts.

Apart from performance implications, scanning the template here makes
IWYU report all template parameters as if specified explicitly by the
user (effectively ignoring default template params). For example,
std::allocator would get reported as a full-use during a call to
push_back() because it is defined outside of the std::vector class.

Closes #533

iwyu.cc

@@ -2583,23 +2583,6 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
     ASTNode* ast_node = current_ast_node();
     ast_node->set_in_forward_declare_context(false);
 
-    // For method calls it doesn't matter if a method is defined inside
-    // a class or outside of it.  We detect out-of-class method calls with
-    // the pattern
-    //
-    //   CallExpr
-    //   `-CXXMethodDecl
-    //     `-NestedNameSpecifier
-    //
-    // and skip traversing method qualifier as in-class methods don't have it.
-    if (const CXXMethodDecl* parent = ast_node->GetParentAs<CXXMethodDecl>()) {
-      if ((nns == parent->getQualifier()) &&
-          ast_node->AncestorIsA<CallExpr>(2)) {
-        VERRS(7) << "Skipping traversal of CXXMethodDecl qualifier "
-                 << PrintableNestedNameSpecifier(nns) << "\n";
-        return false;
-      }
-    }
     return true;
   }
 
@@ -3000,8 +2983,27 @@ class InstantiatedTemplateVisitor
   bool TraverseTemplateSpecializationTypeHelper(
       const clang::TemplateSpecializationType* type) {
     if (CanIgnoreCurrentASTNode())  return true;
-    if (CanForwardDeclareType(current_ast_node()))
-      current_ast_node()->set_in_forward_declare_context(true);
+
+    // Skip the template traversal if this occurrence of the template name is
+    // just a class qualifier for an out of line method, as opposed to an object
+    // instantiation, where the templated code would need to be inspected.
+    //
+    // Class<T,U>::method() {
+    // |-Type---^
+    // |-NNS------^
+    // |-CXXMethodDecl--^
+    ASTNode* ast_node = current_ast_node();
+    if (const auto* nns = ast_node->GetParentAs<NestedNameSpecifier>()) {
+      if (nns->getAsType() == type) {
+        if (const auto* method = ast_node->GetAncestorAs<CXXMethodDecl>(2)) {
+          CHECK_(nns == method->getQualifier());
+          return true;
+        }
+      }
+    }
+
+    if (CanForwardDeclareType(ast_node))
+      ast_node->set_in_forward_declare_context(true);
     return TraverseDataAndTypeMembersOfClassHelper(type);
   }
 

tests/cxx/badinc.cc

@@ -1520,9 +1520,7 @@ int main() {
   // IWYU: std::vector is...*<vector>
   // IWYU: I2_Enum is...*badinc-i2.h
   std::vector<I2_Enum> local_enum_vector;
-  // TODO: In C++03 and earlier I2_Enum is required below. The fact that this
-  // depends on standard version indicates a bug in our template handling, as
-  // the instantiation chain is different in old vs. new standard libraries.
+  // IWYU: I2_Enum is...*badinc-i2.h
   // IWYU: std::vector is...*<vector>
   // IWYU: I21 is...*badinc-i2.h
   local_enum_vector.push_back(I21);
@@ -1843,9 +1841,16 @@ int main() {
   H_TemplateFunction<I1_Class*>(&i1_class);
   H_TemplateFunction(&i1_class);
   // IWYU: I22 is...*badinc-i2.h
+  // IWYU: I1_Enum is...*badinc-i1.h
   h_templateclass2.static_out_of_line(I22);
   // IWYU: I22 is...*badinc-i2.h
+  // IWYU: I1_Enum is...*badinc-i1.h
+  h_templateclass2.static_inline(I22);
+  // IWYU: I22 is...*badinc-i2.h
   h_templateclass2.h_nested_struct.tplnested(I22);
+  // TODO: I1_Enum should be reported here as a full use but isn't,
+  // because we are not properly handling the dependent type FOO in
+  // the nested struct.
   // IWYU: I22 is...*badinc-i2.h
   h_templateclass2.h_nested_struct.static_tplnested(I22);
   // This should not cause warnings for the i2_class destructor

tests/cxx/badinc.h

@@ -203,8 +203,15 @@ class H_TemplateClass {
     I2_Class i2_class;
     (void)i2_class;
   }
+
   // IWYU: I2_Enum is...*badinc-i2.h
   static FOO static_out_of_line(I2_Enum i2_enum);
+
+  // IWYU: I2_Enum is...*badinc-i2.h
+  static FOO static_inline(I2_Enum i2_enum) {
+    return static_out_of_line(i2_enum);
+  }
+
   // IWYU: I2_Enum is...*badinc-i2.h
   static FOO static_never_defined(I2_Enum i2_enum);
   struct H_TplNestedStruct {

tests/cxx/out_of_line-dep-char.h

@@ -0,0 +1,14 @@
+//===--- out_of_line-dep-char.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.
+//
+//===----------------------------------------------------------------------===//
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_CHAR_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_CHAR_H_
+
+void Dependent(char);
+
+#endif  // INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_CHAR_H_

tests/cxx/out_of_line-dep-int.h

@@ -0,0 +1,14 @@
+//===--- out_of_line-dep-int.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.
+//
+//===----------------------------------------------------------------------===//
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_INT_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_INT_H_
+
+void Dependent(int);
+
+#endif  // INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_INT_H_

tests/cxx/out_of_line-dep.h

@@ -0,0 +1,15 @@
+//===--- out_of_line-dep.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.
+//
+//===----------------------------------------------------------------------===//
+#ifndef INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_H_
+#define INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_H_
+
+#include "out_of_line-dep-int.h"
+#include "out_of_line-dep-char.h"
+
+#endif  // INCLUDE_WHAT_YOU_USE_TESTS_CXX_OUT_OF_LINE_DEP_H_

tests/cxx/out_of_line.cc

@@ -0,0 +1,78 @@
+//===--- out_of_line.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.
+//
+//===----------------------------------------------------------------------===//
+
+#include "out_of_line-dep.h"
+
+template <class T, class U = char>
+struct Class {
+  using Type = T;
+
+  void MethodIn() { Dependent(T()); }
+  void MethodOut();
+
+  static void MethodStaticIn() { Dependent(T()); }
+  static void MethodStaticOut();
+};
+
+template <class T, class U>
+void Class<T, U>::MethodOut() {
+  Dependent(T());
+}
+
+template <class T, class U>
+void Class<T, U>::MethodStaticOut() {
+  Dependent(T());
+}
+
+template <>
+void Class<int, int>::MethodOut() {
+  // IWYU: Dependent is.*dep-int.h
+  Dependent(Type());
+}
+
+
+int main()
+{
+    //Inline template method
+    Class<int> c;
+    // IWYU: Dependent is.*dep-int.h
+    c.MethodIn();
+
+    //Out-of-line template method
+    Class<int> c2;
+    // IWYU: Dependent is.*dep-int.h
+    c2.MethodOut();
+
+    //A explicit specialization of MethodOut(). It does not
+    //require Dependent(int) here, because it must be already
+    //available at the definition.
+    Class<int, int> c3;
+    c3.MethodOut();
+
+    //Inline static method
+    // IWYU: Dependent is.*dep-int.h
+    Class<int>::MethodStaticIn();
+
+    //Out-of-line static method
+    // IWYU: Dependent is.*dep-int.h
+    Class<int>::MethodStaticOut();
+}
+
+/**** IWYU_SUMMARY
+
+tests/cxx/out_of_line.cc should add these lines:
+#include "out_of_line-dep-int.h"
+
+tests/cxx/out_of_line.cc should remove these lines:
+- #include "out_of_line-dep.h"  // lines XX-XX
+
+The full include-list for tests/cxx/out_of_line.cc:
+#include "out_of_line-dep-int.h"  // for Dependent
+
+***** IWYU_SUMMARY */