[change-namepsace] make it possible to whitelist symbols so they don't get updated.

[ioeric]

• removed a debug message

[hokein]

LGTM.

change-namespace/tool/ClangChangeNamespace.cpp

78

Maybe consider create a command-line option for it? It will make the tool more pluggable IMO.

[ioeric]

• Make the whitelist an option.

[ioeric]

PTAL

change-namespace/tool/ClangChangeNamespace.cpp

78

Good idea. Done.

[hokein]

change-namespace/tool/ClangChangeNamespace.cpp

82

Instead std::vector<std::string>, maybe std::vector<llvm::Regex> is better, with that we don't need to do transform stuff in ChangeNamespaceTool.

test/change-namespace/Inputs/white-list.txt

1 ↗

(On Diff #89637)

As this is only one-line file, I'd create this file in whitelist lint test like echo XX > whitelist.txt to avoid adding a new file in test.

unittests/change-namespace/ChangeNamespaceTests.cpp

41–42

/*WhiteListedSymbolPatterns*/{} is enough.

[ioeric]

• Addressed comments.

change-namespace/tool/ClangChangeNamespace.cpp

82

I'd like ChangeNamespaceTool to own the Regex vector and make sure it is not shared since Regex is not thread safe.

change-namespace/ChangeNamespace.h

 //   }  // x
 // FIXME: support moving typedef, enums across namespaces.
 class ChangeNamespaceTool : public ast_matchers::MatchFinder::MatchCallback {
 public:
   // Moves code in the old namespace `OldNs` to the new namespace `NewNs` in
   // files matching `FilePattern`.
   ChangeNamespaceTool(
       llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern,
+      llvm::ArrayRef<std::string> WhiteListedSymbolPatterns,
       std::map<std::string, tooling::Replacements> *FileToReplacements,
       llvm::StringRef FallbackStyle = "LLVM");
 
   void registerMatchers(ast_matchers::MatchFinder *Finder);
 
   void run(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
   // Moves the changed code in old namespaces but leaves class forward
private:
   llvm::SmallPtrSet<const NamespaceAliasDecl *, 8> NamespaceAliasDecls;
   // TypeLocs of CXXCtorInitializer. Types of CXXCtorInitializers do not need to
   // be fixed.
   llvm::SmallVector<TypeLoc, 8> BaseCtorInitializerTypeLocs;
   // Since a DeclRefExpr for a function call can be matched twice (one as
   // CallExpr and one as DeclRefExpr), we record all DeclRefExpr's that have
   // been processed so that we don't handle them twice.
   llvm::SmallPtrSet<const clang::DeclRefExpr*, 16> ProcessedFuncRefs;
+  // Patterns of symbol names whose references are not expected to be updated
+  // when changing namespaces around them.
+  std::vector<llvm::Regex> WhiteListedSymbolRegexes;
 };
 
 } // namespace change_namespace
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CHANGE_NAMESPACE_CHANGENAMESPACE_H

change-namespace/ChangeNamespace.cpp

 AST_MATCHER(EnumDecl, isScoped) {
     return Node.isScoped();
 }
 
 } // anonymous namespace
 
 ChangeNamespaceTool::ChangeNamespaceTool(
     llvm::StringRef OldNs, llvm::StringRef NewNs, llvm::StringRef FilePattern,
+    llvm::ArrayRef<std::string> WhiteListedSymbolPatterns,
     std::map<std::string, tooling::Replacements> *FileToReplacements,
     llvm::StringRef FallbackStyle)
     : FallbackStyle(FallbackStyle), FileToReplacements(*FileToReplacements),
       OldNamespace(OldNs.ltrim(':')), NewNamespace(NewNs.ltrim(':')),
       FilePattern(FilePattern), FilePatternRE(FilePattern) {
   FileToReplacements->clear();
   llvm::SmallVector<llvm::StringRef, 4> OldNsSplitted;
   llvm::SmallVector<llvm::StringRef, 4> NewNsSplitted;
   llvm::StringRef(OldNamespace).split(OldNsSplitted, "::");
   llvm::StringRef(NewNamespace).split(NewNsSplitted, "::");
   // Calculates `DiffOldNamespace` and `DiffNewNamespace`.
   while (!OldNsSplitted.empty() && !NewNsSplitted.empty() &&
          OldNsSplitted.front() == NewNsSplitted.front()) {
     OldNsSplitted.erase(OldNsSplitted.begin());
     NewNsSplitted.erase(NewNsSplitted.begin());
   }
   DiffOldNamespace = joinNamespaces(OldNsSplitted);
   DiffNewNamespace = joinNamespaces(NewNsSplitted);
+
+  for (const auto &Pattern : WhiteListedSymbolPatterns)
+    WhiteListedSymbolRegexes.emplace_back(Pattern);
 }
 
 void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   std::string FullOldNs = "::" + OldNamespace;
   // Prefix is the outer-most namespace in DiffOldNamespace. For example, if the
   // OldNamespace is "a::b::c" and DiffOldNamespace is "b::c", then Prefix will
   // be "a::b". Declarations in this namespace will not be visible in the new
   // namespace. If DiffOldNamespace is empty, Prefix will be a invalid name "-".
void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext(
   const std::string NewNs = (NewNamespace + Postfix).str();
 
   llvm::StringRef NestedName = Lexer::getSourceText(
       CharSourceRange::getTokenRange(
           Result.SourceManager->getSpellingLoc(Start),
           Result.SourceManager->getSpellingLoc(End)),
       *Result.SourceManager, Result.Context->getLangOpts());
   std::string FromDeclName = FromDecl->getQualifiedNameAsString();
+  for (llvm::Regex &RE : WhiteListedSymbolRegexes)
+    if (RE.match(FromDeclName))
+      return;
   std::string ReplaceName =
       getShortestQualifiedNameInNamespace(FromDeclName, NewNs);
   // Checks if there is any using namespace declarations that can shorten the
   // qualified name.
   for (const auto *UsingNamespace : UsingNamespaceDecls) {
     if (!isDeclVisibleAtLocation(*Result.SourceManager, UsingNamespace, DeclCtx,
                                  Start))
       continue;

change-namespace/tool/ClangChangeNamespace.cpp

cl::opt<bool>
     DumpYAML("dump_result",
          cl::desc("Dump new file contents in YAML, if specified."),
          cl::cat(ChangeNamespaceCategory));
 
 cl::opt<std::string> Style("style",
                            cl::desc("The style name used for reformatting."),
                            cl::init("LLVM"), cl::cat(ChangeNamespaceCategory));
 
+cl::opt<std::string> WhiteListFile(
+    "whitelist_file",
+    cl::desc("A file containing regexes of symbol names that are not expected "
+             "to be updated when changing namespaces around them."),
+    cl::init(""), cl::cat(ChangeNamespaceCategory));
+
+llvm::ErrorOr<std::vector<std::string>> GetWhiteListedSymbolPatterns() {
+  llvm::SmallVector<StringRef, 8> Lines;
+  if (!WhiteListFile.empty()) {
+    llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> File =
+        llvm::MemoryBuffer::getFile(WhiteListFile);
+    if (!File)
+      return File.getError();
+    llvm::StringRef Content = File.get()->getBuffer();
+    Content.split(Lines, '\n', /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  }
+  return std::vector<std::string>(Lines.begin(), Lines.end());
+}
+
 } // anonymous namespace
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   tooling::CommonOptionsParser OptionsParser(argc, argv,
                                              ChangeNamespaceCategory);
   const auto &Files = OptionsParser.getSourcePathList();
   tooling::RefactoringTool Tool(OptionsParser.getCompilations(), Files);
+  llvm::ErrorOr<std::vector<std::string>> WhiteListPatterns =
+      GetWhiteListedSymbolPatterns();
+  if (!WhiteListPatterns) {
+    llvm::errs() << "Failed to open whitelist file " << WhiteListFile << ". "
+                 << WhiteListPatterns.getError().message() << "\n";
+    return 1;
+  }
   change_namespace::ChangeNamespaceTool NamespaceTool(
-      OldNamespace, NewNamespace, FilePattern, &Tool.getReplacements(), Style);
+      OldNamespace, NewNamespace, FilePattern, *WhiteListPatterns,
+      &Tool.getReplacements(), Style);
   ast_matchers::MatchFinder Finder;
   NamespaceTool.registerMatchers(&Finder);
   std::unique_ptr<tooling::FrontendActionFactory> Factory =
       tooling::newFrontendActionFactory(&Finder);
 
   if (int Result = Tool.run(Factory.get()))
     return Result;
   LangOptions DefaultLangOptions;

test/change-namespace/Inputs/fake-std.h

+namespace std {
+  class STD {};
+}
+
+using namespace std;

test/change-namespace/white-list.cpp

+// RUN: echo "^std::.*$" > %T/white-list.txt
+// RUN: clang-change-namespace -old_namespace "na::nb" -new_namespace "x::y" --file_pattern ".*" --whitelist_file %T/white-list.txt %s -- | sed 's,// CHECK.*,,' | FileCheck %s
+
+#include "Inputs/fake-std.h"
+
+// CHECK: namespace x {
+// CHECK-NEXT: namespace y {
+namespace na {
+namespace nb {
+void f() {
+  std::STD x1;
+  STD x2;
+// CHECK: {{^}}  std::STD x1;{{$}}
+// CHECK-NEXT: {{^}}  STD x2;{{$}}
+}
+// CHECK: } // namespace y
+// CHECK-NEXT: } // namespace x
+}
+}

unittests/change-namespace/ChangeNamespaceTests.cpp

 class ChangeNamespaceTest : public ::testing::Test {
 public:
   std::string runChangeNamespaceOnCode(llvm::StringRef Code) {
     clang::RewriterTestContext Context;
     clang::FileID ID = Context.createInMemoryFile(FileName, Code);
 
     std::map<std::string, tooling::Replacements> FileToReplacements;
     change_namespace::ChangeNamespaceTool NamespaceTool(
-        OldNamespace, NewNamespace, FilePattern, &FileToReplacements);
+        OldNamespace, NewNamespace, FilePattern,
+        /*WhiteListedSymbolPatterns*/ {}, &FileToReplacements);
     ast_matchers::MatchFinder Finder;
     NamespaceTool.registerMatchers(&Finder);
     std::unique_ptr<tooling::FrontendActionFactory> Factory =
         tooling::newFrontendActionFactory(&Finder);
     if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, {"-std=c++11"},
                                         FileName))
       return "";
     formatAndApplyAllReplacements(FileToReplacements, Context.Rewrite);