Skip ParmVarDecl nodes without a name.

BUG=598141

Review-Url: https://codereview.chromium.org/2246263002
Cr-Commit-Position: refs/heads/master@{#414877}

docs/clang_tool_refactoring.md

@@ -172,7 +172,7 @@ that is to `return 1` from the `main()` function of the clang tool.
 Synposis:
 
 ```shell
-test_tool.py <tool name>
+tools/clang/scripts/test_tool.py <tool name>
 ```
 
 The name of the tool binary and the subdirectory for the tool in

tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

@@ -272,7 +272,7 @@ bool IsProbablyConst(const clang::VarDecl& decl,
 }
 
 bool GetNameForDecl(const clang::FunctionDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   name = decl.getName().str();
   name[0] = clang::toUppercase(name[0]);
@@ -287,7 +287,7 @@ bool GetNameForDecl(const clang::FunctionDecl& decl,
 }
 
 bool GetNameForDecl(const clang::EnumConstantDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   StringRef original_name = decl.getName();
 
@@ -314,7 +314,7 @@ bool GetNameForDecl(const clang::EnumConstantDecl& decl,
 }
 
 bool GetNameForDecl(const clang::FieldDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   StringRef original_name = decl.getName();
   bool member_prefix = original_name.startswith(kBlinkFieldPrefix);
@@ -333,13 +333,33 @@ bool GetNameForDecl(const clang::FieldDecl& decl,
 }
 
 bool GetNameForDecl(const clang::VarDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   StringRef original_name = decl.getName();
 
   // Nothing to do for unnamed parameters.
-  if (clang::isa<clang::ParmVarDecl>(decl) && original_name.empty())
+  if (clang::isa<clang::ParmVarDecl>(decl)) {
-    return false;
+    if (original_name.empty())
+      return false;
+
+    // Check if |decl| and |decl.getLocation| are in sync.  We need to skip
+    // out-of-sync ParmVarDecls to avoid renaming buggy ParmVarDecls that
+    // 1) have decl.getLocation() pointing at a parameter declaration without a
+    // name, but 2) have decl.getName() retained from a template specialization
+    // of a method.  See also: https://llvm.org/bugs/show_bug.cgi?id=29145
+    clang::SourceLocation loc =
+        context.getSourceManager().getSpellingLoc(decl.getLocation());
+    auto parents = context.getParents(decl);
+    bool is_child_location_within_parent_source_range = std::all_of(
+        parents.begin(), parents.end(),
+        [&loc](const clang::ast_type_traits::DynTypedNode& parent) {
+          clang::SourceLocation begin = parent.getSourceRange().getBegin();
+          clang::SourceLocation end = parent.getSourceRange().getEnd();
+          return (begin < loc) && (loc < end);
+        });
+    if (!is_child_location_within_parent_source_range)
+      return false;
+  }
 
   // static class members match against VarDecls. Blink style dictates that
   // these should be prefixed with `s_`, so strip that off. Also check for `m_`
@@ -380,14 +400,14 @@ bool GetNameForDecl(const clang::VarDecl& decl,
 }
 
 bool GetNameForDecl(const clang::FunctionTemplateDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   clang::FunctionDecl* templated_function = decl.getTemplatedDecl();
   return GetNameForDecl(*templated_function, context, name);
 }
 
 bool GetNameForDecl(const clang::NamedDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   if (auto* function = clang::dyn_cast<clang::FunctionDecl>(&decl))
     return GetNameForDecl(*function, context, name);
@@ -405,7 +425,7 @@ bool GetNameForDecl(const clang::NamedDecl& decl,
 }
 
 bool GetNameForDecl(const clang::UsingDecl& decl,
-                    const clang::ASTContext& context,
+                    clang::ASTContext& context,
                     std::string& name) {
   assert(decl.shadow_size() > 0);
 

tools/clang/rewrite_to_chrome_style/tests/template-expected.cc

@@ -140,4 +140,25 @@ struct Class2 {
 
 }  // namespace test_template_arg_is_method_template_in_member_context
 
+namespace test_unnamed_arg {
+
+template <typename T>
+class Class {
+ public:
+  // Test for https://crbug.com/598141 - shouldn't rewrite
+  //    ...int);
+  // into
+  //    ...intdata_size;
+  void F(int);
+};
+
+template <typename T>
+void Class<T>::F(int data_size){};
+
+void Foo() {
+  Class<char>().F(123);
+};
+
+}  // namespace test_unnamed_arg
+
 }  // namespace blink

tools/clang/rewrite_to_chrome_style/tests/template-original.cc

@@ -140,4 +140,25 @@ struct Class2 {
 
 }  // namespace test_template_arg_is_method_template_in_member_context
 
+namespace test_unnamed_arg {
+
+template <typename T>
+class Class {
+ public:
+  // Test for https://crbug.com/598141 - shouldn't rewrite
+  //    ...int);
+  // into
+  //    ...intdata_size;
+  void f(int);
+};
+
+template <typename T>
+void Class<T>::f(int dataSize){};
+
+void foo() {
+  Class<char>().f(123);
+};
+
+}  // namespace test_unnamed_arg
+
 }  // namespace blink