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

[Viktoriia Bakalova via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG45659b3bd98e: [include-cleaner] Remove filtering from 
UsingDecl visit. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138821/new/

https://reviews.llvm.org/D138821

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


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -26,7 +26,6 @@
 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 @@
   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 @@
     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 differences, we print the entire referencing code once.
@@ -129,19 +124,32 @@
 }
 
 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) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ 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 @@
 
 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 @@
   }
 
 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 @@
     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 @@
 } // 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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits