[clang-tools-extra] 45659b3 - [include-cleaner] Remove filtering from UsingDecl visit.

[Viktoriia Bakalova via cfe-commits]

Author: Viktoriia Bakalova
Date: 2022-12-08T10:23:55Z
New Revision: 45659b3bd98ea3b8ce13516bcf719669b934b9ba

URL: https://github.com/llvm/llvm-project/commit/45659b3bd98ea3b8ce13516bcf719669b934b9ba
DIFF: https://github.com/llvm/llvm-project/commit/45659b3bd98ea3b8ce13516bcf719669b934b9ba.diff

LOG: [include-cleaner] Remove filtering from UsingDecl visit.

Removes filtering from the VisitUsingDecl method for implementation files.

Differential Revision: https://reviews.llvm.org/D138821

Added: 
    

Modified: 
    clang-tools-extra/include-cleaner/lib/WalkAST.cpp
    clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index 3d338cd59b282..7fb3f5697b7ed 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -16,7 +16,6 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 
@@ -27,10 +26,6 @@ using DeclCallback =
 
 class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
   DeclCallback Callback;
-  // Whether we're traversing declarations coming from a header file.
-  // This helps figure out whether certain symbols can be assumed as unused
-  // (e.g. overloads brought into an implementation file, but not used).
-  bool IsHeader = false;
 
   bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
     // For using-templates, only mark the alias.
@@ -50,8 +45,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
   }
 
 public:
-  ASTWalker(DeclCallback Callback, bool IsHeader)
-      : Callback(Callback), IsHeader(IsHeader) {}
+  ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
     report(DRE->getLocation(), DRE->getFoundDecl());
@@ -82,10 +76,6 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
     for (const auto *Shadow : UD->shadows()) {
       auto *TD = Shadow->getTargetDecl();
       auto IsUsed = TD->isUsed() || TD->isReferenced();
-      // We ignore unused overloads inside implementation files, as the ones in
-      // headers might still be used by the dependents of the header.
-      if (!IsUsed && !IsHeader)
-        continue;
       report(UD->getLocation(), TD,
              IsUsed ? RefType::Explicit : RefType::Ambiguous);
     }
@@ -151,14 +141,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
 } // namespace
 
 void walkAST(Decl &Root, DeclCallback Callback) {
-  auto &AST = Root.getASTContext();
-  auto &SM = AST.getSourceManager();
-  // If decl isn't written in main file, assume it's coming from an include,
-  // hence written in a header.
-  bool IsRootedAtHeader =
-      AST.getLangOpts().IsHeaderFile ||
-      !SM.isWrittenInMainFile(SM.getSpellingLoc(Root.getLocation()));
-  ASTWalker(Callback, IsRootedAtHeader).TraverseDecl(&Root);
+  ASTWalker(Callback).TraverseDecl(&Root);
 }
 
 } // namespace clang::include_cleaner

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 17d13018d1522..d9cf84dc7abe9 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -26,7 +26,6 @@ namespace clang::include_cleaner {
 namespace {
 
 // Specifies a test of which symbols are referenced by a piece of code.
-// If `// c++-header` is present, treats referencing code as a header file.
 // Target should contain points annotated with the reference kind.
 // Example:
 //   Target:      int $explicit^foo();
@@ -41,8 +40,6 @@ void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
   Inputs.ExtraArgs.push_back("-std=c++17");
-  if (Referencing.code().contains("// c++-header\n"))
-    Inputs.ExtraArgs.push_back("-xc++-header");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -88,12 +85,10 @@ void testWalk(llvm::StringRef TargetCode, llvm::StringRef ReferencingCode) {
     auto RTStr = llvm::to_string(RT);
     for (auto Expected : Target.points(RTStr))
       if (!llvm::is_contained(ReferencedOffsets[RT], Expected))
-        DiagnosePoint("location not marked used with type " + RTStr,
-                      Expected);
+        DiagnosePoint("location not marked used with type " + RTStr, Expected);
     for (auto Actual : ReferencedOffsets[RT])
       if (!llvm::is_contained(Target.points(RTStr), Actual))
-        DiagnosePoint("location unexpectedly used with type " + RTStr,
-                      Actual);
+        DiagnosePoint("location unexpectedly used with type " + RTStr, Actual);
   }
 
   // If there were any 
diff erences, we print the entire referencing code once.
@@ -129,19 +124,32 @@ TEST(WalkAST, Alias) {
 }
 
 TEST(WalkAST, Using) {
-  // Make sure we ignore unused overloads.
+  // We should report unused overloads as ambiguous.
   testWalk(R"cpp(
     namespace ns {
-      void $explicit^x(); void x(int); void x(char);
+      void $explicit^x(); void $ambiguous^x(int); void $ambiguous^x(char);
     })cpp",
            "using ns::^x; void foo() { x(); }");
-  // We should report unused overloads if main file is a header.
   testWalk(R"cpp(
     namespace ns {
       void $ambiguous^x(); void $ambiguous^x(int); void $ambiguous^x(char);
     })cpp",
-           "// c++-header\n using ns::^x;");
+           "using ns::^x;");
   testWalk("namespace ns { struct S; } using ns::$explicit^S;", "^S *s;");
+
+  testWalk(R"cpp(
+    namespace ns {
+      template<class T>
+      class $ambiguous^Y {};
+    })cpp",
+           "using ns::^Y;");
+  testWalk(R"cpp(
+    namespace ns {
+      template<class T>
+      class Y {};
+    }
+    using ns::$explicit^Y;)cpp",
+           "^Y<int> x;");
 }
 
 TEST(WalkAST, Namespaces) {