Don't rename references to functions that originate from template args.

tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Changes Blink-style names to Chrome-style names. Currently transforms:
 //   fields:
 //     int m_operationCount => int operation_count_
 //   variables (including parameters):
 //     int mySuperVariable => int my_super_variable
 //   constants:
@@ skipping 30 matching lines @@
 #endif
 
 using namespace clang::ast_matchers;
 using clang::tooling::CommonOptionsParser;
 using clang::tooling::Replacement;
 using clang::tooling::Replacements;
 using llvm::StringRef;
 
 namespace {
 
-// Hack: prevent the custom isDefaulted() from conflicting with the one defined
-// in newer revisions of Clang.
-namespace internal_hack {
-
-// This is available in newer clang revisions... but alas, Chrome has not rolled
-// that far yet.
-AST_MATCHER(clang::FunctionDecl, isDefaulted) {

[dcheng, 2016/03/17 04:03:39]

No longer needed, yay!

-  return Node.isDefaulted();
-}
-
-}  // namespace internal_hack
-
 const char kBlinkFieldPrefix[] = "m_";
 const char kBlinkStaticMemberPrefix[] = "s_";
 const char kGeneratedFileRegex[] = "^gen/|/gen/";
 
 AST_MATCHER(clang::FunctionDecl, isOverloadedOperator) {
   return Node.isOverloadedOperator();
 }
 
 AST_MATCHER_P(clang::FunctionTemplateDecl,
               templatedDecl,
@@ skipping 123 matching lines @@
 AST_MATCHER(clang::CXXMethodDecl, isBlacklistedMethod) {
   return IsBlacklistedMethod(Node);
 }
 
 // Helper to convert from a camelCaseName to camel_case_name. It uses some
 // heuristics to try to handle acronyms in camel case names correctly.
 std::string CamelCaseToUnderscoreCase(StringRef input) {
   std::string output;
   bool needs_underscore = false;
   bool was_lowercase = false;
-  bool was_number = false;

[dcheng, 2016/03/17 04:03:39]

These were giving me warnings.

   bool was_uppercase = false;
   bool first_char = true;
   // Iterate in reverse to minimize the amount of backtracking.
   for (const unsigned char* i = input.bytes_end() - 1; i >= input.bytes_begin();
        --i) {
     char c = *i;
     bool is_lowercase = clang::isLowercase(c);
-    bool is_number = clang::isDigit(c);
     bool is_uppercase = clang::isUppercase(c);
     c = clang::toLowercase(c);
     // Transitioning from upper to lower case requires an underscore. This is
     // needed to handle names with acronyms, e.g. handledHTTPRequest needs a '_'
     // in 'dH'. This is a complement to the non-acronym case further down.
     if (was_uppercase && is_lowercase)
       needs_underscore = true;
     if (needs_underscore) {
       output += '_';
       needs_underscore = false;
     }
     output += c;
     // Handles the non-acronym case: transitioning from lower to upper case
     // requires an underscore when emitting the next character, e.g. didLoad
     // needs a '_' in 'dL'.
     if (!first_char && was_lowercase && is_uppercase)
       needs_underscore = true;
     was_lowercase = is_lowercase;
-    was_number = is_number;
     was_uppercase = is_uppercase;
     first_char = false;
   }
   std::reverse(output.begin(), output.end());
   return output;
 }
 
 bool IsProbablyConst(const clang::VarDecl& decl,
                      const clang::ASTContext& context) {
   clang::QualType type = decl.getType();
@@ skipping 149 matching lines @@
 
 bool GetNameForDecl(const clang::FunctionTemplateDecl& decl,
                     const clang::ASTContext& context,
                     std::string& name) {
   clang::FunctionDecl* templated_function = decl.getTemplatedDecl();
   if (auto* method = clang::dyn_cast<clang::CXXMethodDecl>(templated_function))
     return GetNameForDecl(*method, context, name);
   return GetNameForDecl(*templated_function, context, name);
 }
 
+bool GetNameForDecl(const clang::NamedDecl& decl,
+                    const clang::ASTContext& context,
+                    std::string& name) {
+  // Note: CXXMethodDecl must be checked before FunctionDecl, because
+  // CXXMethodDecl is derived from FunctionDecl.
+  if (auto* method = clang::dyn_cast<clang::CXXMethodDecl>(&decl))
+    return GetNameForDecl(*method, context, name);
+  if (auto* function = clang::dyn_cast<clang::FunctionDecl>(&decl))
+    return GetNameForDecl(*function, context, name);
+  if (auto* var = clang::dyn_cast<clang::VarDecl>(&decl))
+    return GetNameForDecl(*var, context, name);
+  if (auto* field = clang::dyn_cast<clang::FieldDecl>(&decl))
+    return GetNameForDecl(*field, context, name);
+  if (auto* function_template =
+          clang::dyn_cast<clang::FunctionTemplateDecl>(&decl))
+    return GetNameForDecl(*function_template, context, name);
+  if (auto* enumc = clang::dyn_cast<clang::EnumConstantDecl>(&decl))
+    return GetNameForDecl(*enumc, context, name);
+
+  return false;
+}
+
 bool GetNameForDecl(const clang::UsingDecl& decl,
                     const clang::ASTContext& context,
                     std::string& name) {
   assert(decl.shadow_size() > 0);

[dcheng, 2016/03/17 04:03:39]

I did some refactoring in this area when trying to fix another bug and gave up.
I figured that the reorganization of code doesn't really hurt and might be
useful later, so I just left it.

 
   // If a using declaration's targeted declaration is a set of overloaded
   // functions, it can introduce multiple shadowed declarations. Just using the
   // first one is OK, since overloaded functions have the same name, by
   // definition.
-  clang::NamedDecl* shadowed_name = decl.shadow_begin()->getTargetDecl();
-  // Note: CXXMethodDecl must be checked before FunctionDecl, because
-  // CXXMethodDecl is derived from FunctionDecl.
-  if (auto* method = clang::dyn_cast<clang::CXXMethodDecl>(shadowed_name))
-    return GetNameForDecl(*method, context, name);
-  if (auto* function = clang::dyn_cast<clang::FunctionDecl>(shadowed_name))
-    return GetNameForDecl(*function, context, name);
-  if (auto* var = clang::dyn_cast<clang::VarDecl>(shadowed_name))
-    return GetNameForDecl(*var, context, name);
-  if (auto* field = clang::dyn_cast<clang::FieldDecl>(shadowed_name))
-    return GetNameForDecl(*field, context, name);
-  if (auto* function_template =
-          clang::dyn_cast<clang::FunctionTemplateDecl>(shadowed_name))
-    return GetNameForDecl(*function_template, context, name);
-  if (auto* enumc = clang::dyn_cast<clang::EnumConstantDecl>(shadowed_name))
-    return GetNameForDecl(*enumc, context, name);
-
-  return false;
+  return GetNameForDecl(*decl.shadow_begin()->getTargetDecl(), context, name);
 }
 
 template <typename Type>
 struct TargetNodeTraits;
 
 template <>
 struct TargetNodeTraits<clang::NamedDecl> {
-  static const char kName[];
   static clang::SourceLocation GetLoc(const clang::NamedDecl& decl) {
     return decl.getLocation();
   }
-  static const char* GetType() { return "NamedDecl"; }
+  static const char* GetName() { return "decl"; }
 };
-const char TargetNodeTraits<clang::NamedDecl>::kName[] = "decl";

[dcheng, 2016/03/17 04:03:39]

Ditto: these were incidental cleanups when I was trying to fix
https://crbug.com/584408

 
 template <>
 struct TargetNodeTraits<clang::MemberExpr> {
-  static const char kName[];
   static clang::SourceLocation GetLoc(const clang::MemberExpr& expr) {
     return expr.getMemberLoc();
   }
-  static const char* GetType() { return "MemberExpr"; }
+  static const char* GetName() { return "expr"; }
 };
-const char TargetNodeTraits<clang::MemberExpr>::kName[] = "expr";
 
 template <>
 struct TargetNodeTraits<clang::DeclRefExpr> {
-  static const char kName[];
   static clang::SourceLocation GetLoc(const clang::DeclRefExpr& expr) {
     return expr.getLocation();
   }
-  static const char* GetType() { return "DeclRefExpr"; }
+  static const char* GetName() { return "expr"; }
 };
-const char TargetNodeTraits<clang::DeclRefExpr>::kName[] = "expr";
 
 template <>
 struct TargetNodeTraits<clang::CXXCtorInitializer> {
-  static const char kName[];
   static clang::SourceLocation GetLoc(const clang::CXXCtorInitializer& init) {
     assert(init.isWritten());
     return init.getSourceLocation();
   }
-  static const char* GetType() { return "CXXCtorInitializer"; }
+  static const char* GetName() { return "initializer"; }
 };
-const char TargetNodeTraits<clang::CXXCtorInitializer>::kName[] = "initializer";
 
 template <typename DeclNode, typename TargetNode>
 class RewriterBase : public MatchFinder::MatchCallback {
  public:
   explicit RewriterBase(Replacements* replacements)
       : replacements_(replacements) {}
 
   void run(const MatchFinder::MatchResult& result) override {
     const DeclNode* decl = result.Nodes.getNodeAs<DeclNode>("decl");
     // If false, there's no name to be renamed.
@@ skipping 14 matching lines @@
     }
     clang::ASTContext* context = result.Context;
     std::string new_name;
     if (!GetNameForDecl(*decl, *context, new_name))
       return;  // If false, the name was not suitable for renaming.
     llvm::StringRef old_name = decl->getName();
     if (old_name == new_name)
       return;
     clang::SourceLocation loc = TargetNodeTraits<TargetNode>::GetLoc(
         *result.Nodes.getNodeAs<TargetNode>(
-            TargetNodeTraits<TargetNode>::kName));
+            TargetNodeTraits<TargetNode>::GetName()));
     clang::CharSourceRange range = clang::CharSourceRange::getTokenRange(loc);
     replacements_->emplace(*result.SourceManager, range, new_name);
     replacement_names_.emplace(old_name.str(), std::move(new_name));
   }
 
   const std::unordered_map<std::string, std::string>& replacement_names()
       const {
     return replacement_names_;
   }
 
@@ skipping 81 matching lines @@
   // matches |x| in if (x).
   auto member_matcher = id(
       "expr",
       memberExpr(
           member(field_decl_matcher),
           // Needed to avoid matching member references in functions (which will
           // be an ancestor of the member reference) synthesized by the
           // compiler, such as a synthesized copy constructor.
           // This skips explicitly defaulted functions as well, but that's OK:
           // there's nothing interesting to rewrite in those either.
-          unless(hasAncestor(functionDecl(internal_hack::isDefaulted())))));
+          unless(hasAncestor(functionDecl(isDefaulted())))));
   auto decl_ref_matcher = id("expr", declRefExpr(to(var_decl_matcher)));
   auto enum_member_ref_matcher =
       id("expr", declRefExpr(to(enum_member_decl_matcher)));
 
   MemberRewriter member_rewriter(&replacements);
   match_finder.addMatcher(member_matcher, &member_rewriter);
 
   DeclRefRewriter decl_ref_rewriter(&replacements);
   match_finder.addMatcher(decl_ref_matcher, &decl_ref_rewriter);
 
@@ skipping 31 matching lines @@
               isOverloadedOperator())),
           in_blink_namespace));
   FunctionDeclRewriter function_decl_rewriter(&replacements);
   match_finder.addMatcher(function_decl_matcher, &function_decl_rewriter);
 
   // Non-method function references ========
   // Given
   //   f();
   //   void (*p)() = &f;
   // matches |f()| and |&f|.
-  auto function_ref_matcher =
-      id("expr", declRefExpr(to(function_decl_matcher)));
+  auto function_ref_matcher = id(
+      "expr", declRefExpr(to(function_decl_matcher),
+                          // Ignore template substitutions
+                          unless(hasAncestor(substNonTypeTemplateParmExpr()))));
   FunctionRefRewriter function_ref_rewriter(&replacements);
   match_finder.addMatcher(function_ref_matcher, &function_ref_rewriter);
 
   // Method declarations ========
   // Given
   //   struct S {
   //     void g();
   //   };
   // matches |g|.
   // For a method to be considered for rewrite, it must not override something
   // that we're not rewriting. Any methods that we would not normally consider
   // but that override something we are rewriting should also be rewritten. So
   // we use includeAllOverriddenMethods() to check these rules not just for the
   // method being matched but for the methods it overrides also.
   auto is_blink_method = includeAllOverriddenMethods(
       allOf(in_blink_namespace, unless(isBlacklistedMethod())));
   auto method_decl_matcher = id(
       "decl",
       cxxMethodDecl(
           unless(anyOf(
               // Overloaded operators have special names and should never be
               // renamed.
               isOverloadedOperator(),
-              // Similarly, constructors, destructors, and conversion functions
+              // Similarly, constructors, destructors, and conversion
+              // functions
               // should not be considered for renaming.
               cxxConstructorDecl(), cxxDestructorDecl(), cxxConversionDecl())),
           // Check this last after excluding things, to avoid
           // asserts about overriding non-blink and blink for the
           // same method.
           is_blink_method));
   MethodDeclRewriter method_decl_rewriter(&replacements);
   match_finder.addMatcher(method_decl_matcher, &method_decl_rewriter);
 
   // Method references in a non-member context ========
   // Given
   //   S s;
   //   s.g();
   //   void (S::*p)() = &S::g;
   // matches |&S::g| but not |s.g()|.
-  auto method_ref_matcher = id("expr", declRefExpr(to(method_decl_matcher)));
+  auto method_ref_matcher = id(
+      "expr", declRefExpr(to(method_decl_matcher),
+                          // Ignore template substitutions.
+                          unless(hasAncestor(substNonTypeTemplateParmExpr()))));
 
   MethodRefRewriter method_ref_rewriter(&replacements);
   match_finder.addMatcher(method_ref_matcher, &method_ref_rewriter);
 
   // Method references in a member context ========
   // Given
   //   S s;
   //   s.g();
   //   void (S::*p)() = &S::g;
   // matches |s.g()| but not |&S::g|.
@@ skipping 81 matching lines @@
   for (const auto& r : replacements) {
     std::string replacement_text = r.getReplacementText().str();
     std::replace(replacement_text.begin(), replacement_text.end(), '\n', '\0');
     llvm::outs() << "r:::" << r.getFilePath() << ":::" << r.getOffset()
                  << ":::" << r.getLength() << ":::" << replacement_text << "\n";
   }
   llvm::outs() << "==== END EDITS ====\n";
 
   return 0;
 }