Handling of DependentScopeDeclRefExpr and CXXDependentScopeMemberExpr nodes.

tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

Index: tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
diff --git a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
index dddccd1741bb968b0c99fec10af2bda8a3a5ec5a..cd4bdae997824190031482b25183dde0fd4d63c1 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
@@ -55,6 +55,14 @@ const clang::ast_matchers::internal::
     VariadicDynCastAllOfMatcher<clang::Expr, clang::UnresolvedMemberExpr>
         unresolvedMemberExpr;
 
+const clang::ast_matchers::internal::
+    VariadicDynCastAllOfMatcher<clang::Expr, clang::DependentScopeDeclRefExpr>
+        dependentScopeDeclRefExpr;
+
+const clang::ast_matchers::internal::
+    VariadicDynCastAllOfMatcher<clang::Expr, clang::CXXDependentScopeMemberExpr>
+        cxxDependentScopeMemberExpr;
+
 AST_MATCHER(clang::FunctionDecl, isOverloadedOperator) {
   return Node.isOverloadedOperator();
 }
@@ -429,6 +437,34 @@ bool GetNameForDecl(const clang::UsingDecl& decl,
   return GetNameForDecl(*decl.shadow_begin()->getTargetDecl(), context, name);
 }
 
+std::string RenameDependentScopeName(const std::string& old_name) {
+  // TODO(lukasza): We don't know if |expr| (i.e. |T::x|) refers to a static
+  // method VS static field VS something else (because we can't tell what |T|
+  // is).  Let's use some simple heuristics for now :-/.
+  std::string new_name;
+  if (old_name.substr(0, 2) == "m_") {
+    new_name = CamelCaseToUnderscoreCase(old_name.substr(2)) + "_";
+  } else {
+    new_name = old_name;
+    new_name[0] = clang::toUppercase(new_name[0]);
+  }

[Łukasz Anforowicz, 2016/08/17 22:45:09]

Not sure if I can/should handle other kinds of renames (e.g. UPPER_CASE ... ->
kUpperCase?).

+  return new_name;
+}
+
+bool GetNameForDecl(const clang::DependentScopeDeclRefExpr& expr,
+                    const clang::ASTContext& context,
+                    std::string& name) {
+  name = RenameDependentScopeName(expr.getDeclName().getAsString());
+  return true;
+}
+
+bool GetNameForDecl(const clang::CXXDependentScopeMemberExpr& expr,
+                    const clang::ASTContext& context,
+                    std::string& name) {
+  name = RenameDependentScopeName(expr.getMember().getAsString());
+  return true;
+}
+
 template <typename Type>
 struct TargetNodeTraits;
 
@@ -438,7 +474,6 @@ struct TargetNodeTraits<clang::NamedDecl> {
     return decl.getLocation();
   }
   static const char* GetName() { return "decl"; }
-  static const char* GetType() { return "NamedDecl"; }

[Łukasz Anforowicz, 2016/08/17 22:45:09]

Deleting - this trait was unused AFAICT.

 };
 
 template <>
@@ -447,7 +482,6 @@ struct TargetNodeTraits<clang::MemberExpr> {
     return expr.getMemberLoc();
   }
   static const char* GetName() { return "expr"; }
-  static const char* GetType() { return "MemberExpr"; }
 };
 
 template <>
@@ -456,7 +490,24 @@ struct TargetNodeTraits<clang::DeclRefExpr> {
     return expr.getLocation();
   }
   static const char* GetName() { return "expr"; }
-  static const char* GetType() { return "DeclRefExpr"; }
+};
+
+template <>
+struct TargetNodeTraits<clang::DependentScopeDeclRefExpr> {
+  static clang::SourceLocation GetLoc(
+      const clang::DependentScopeDeclRefExpr& expr) {
+    return expr.getLocation();
+  }
+  static const char* GetName() { return "decl"; }
+};
+
+template <>
+struct TargetNodeTraits<clang::CXXDependentScopeMemberExpr> {
+  static clang::SourceLocation GetLoc(
+      const clang::CXXDependentScopeMemberExpr& expr) {
+    return expr.getMemberLoc();
+  }
+  static const char* GetName() { return "decl"; }
 };
 
 template <>
@@ -466,7 +517,6 @@ struct TargetNodeTraits<clang::CXXCtorInitializer> {
     return init.getSourceLocation();
   }
   static const char* GetName() { return "initializer"; }
-  static const char* GetType() { return "CXXCtorInitializer"; }
 };
 
 template <>
@@ -475,7 +525,6 @@ struct TargetNodeTraits<clang::UnresolvedLookupExpr> {
     return expr.getNameLoc();
   }
   static const char* GetName() { return "expr"; }
-  static const char* GetType() { return "UnresolvedLookupExpr"; }
 };
 
 template <>
@@ -484,9 +533,62 @@ struct TargetNodeTraits<clang::UnresolvedMemberExpr> {
     return expr.getMemberLoc();
   }
   static const char* GetName() { return "expr"; }
-  static const char* GetType() { return "UnresolvedMemberExpr"; }
 };
 
+template <typename T>
+bool ContainsIdentifier(const T& node) {
+  return node.getIdentifier() != nullptr;
+}
+
+bool ContainsIdentifier(const clang::DependentScopeDeclRefExpr& expr) {
+  return true;
+}
+
+bool ContainsIdentifier(const clang::CXXDependentScopeMemberExpr& expr) {
+  return true;
+}

[Łukasz Anforowicz, 2016/08/17 22:45:09]

ContainsIdentifier, IsDeclarationMacroTokenConcatenated, GetCurrentName
functions were introduced to allow reuse of RewriterBase for
DependentScopeDeclRefExpr and CXXDependentScopeMemberExpr (otherwise
RewriterBase::run body would refer to members not present in
DependentScopeDeclRefExpr and CXXDependentScopeMemberExpr: getIdentifier,
getName, etc.).

+
+bool IsDeclarationMacroTokenConcatenated(
+    const clang::NamedDecl& decl,
+    const clang::SourceManager& source_manager) {
+  clang::SourceLocation decl_loc = decl.getLocation();
+  if (decl_loc.isMacroID()) {
+    // Get the location of the spelling of the declaration. If token pasting
+    // was used this will be in "scratch space" and we don't know how to get
+    // from there back to/ the actual macro with the foo##bar text. So just
+    // don't replace in that case.
+    clang::SourceLocation spell = source_manager.getSpellingLoc(decl_loc);
+    if (strcmp(source_manager.getBufferName(spell), "<scratch space>") == 0)
+      return true;
+  }
+  return false;
+}
+
+bool IsDeclarationMacroTokenConcatenated(
+    const clang::DependentScopeDeclRefExpr& expr,
+    const clang::SourceManager& source_manager) {
+  return false;  // Heuristic - can't realy tell what |expr| will resolve to...
+}
+
+bool IsDeclarationMacroTokenConcatenated(
+    const clang::CXXDependentScopeMemberExpr& expr,
+    const clang::SourceManager& source_manager) {
+  return false;  // Heuristic - can't realy tell what |expr| will resolve to...
+}
+
+template <typename T>
+std::string GetCurrentName(const T& node) {
+  return node.getName();
+}
+
+std::string GetCurrentName(const clang::DependentScopeDeclRefExpr& expr) {
+  return expr.getDeclName().getAsString();
+}
+
+std::string GetCurrentName(const clang::CXXDependentScopeMemberExpr& expr) {
+  return expr.getMember().getAsString();
+}
+
 template <typename DeclNode, typename TargetNode>
 class RewriterBase : public MatchFinder::MatchCallback {
  public:
@@ -494,30 +596,19 @@ class RewriterBase : public MatchFinder::MatchCallback {
       : 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.
-    if (!decl->getIdentifier())
+    const DeclNode& decl = *result.Nodes.getNodeAs<DeclNode>("decl");
+
+    if (!ContainsIdentifier(decl))
+      return;  // If false, there's no name to be renamed.
+    if (IsDeclarationMacroTokenConcatenated(decl, *result.SourceManager))
       return;
-    clang::SourceLocation decl_loc =
-        TargetNodeTraits<clang::NamedDecl>::GetLoc(*decl);
-    if (decl_loc.isMacroID()) {
-      // Get the location of the spelling of the declaration. If token pasting
-      // was used this will be in "scratch space" and we don't know how to get
-      // from there back to/ the actual macro with the foo##bar text. So just
-      // don't replace in that case.
-      clang::SourceLocation spell =
-          result.SourceManager->getSpellingLoc(decl_loc);
-      if (strcmp(result.SourceManager->getBufferName(spell),
-                 "<scratch space>") == 0)
-        return;
-    }
-    clang::ASTContext* context = result.Context;
     std::string new_name;
-    if (!GetNameForDecl(*decl, *context, new_name))
+    if (!GetNameForDecl(decl, *result.Context, new_name))
       return;  // If false, the name was not suitable for renaming.
-    llvm::StringRef old_name = decl->getName();
+    llvm::StringRef old_name = GetCurrentName(decl);
     if (old_name == new_name)
       return;
+
     clang::SourceLocation loc = TargetNodeTraits<TargetNode>::GetLoc(
         *result.Nodes.getNodeAs<TargetNode>(
             TargetNodeTraits<TargetNode>::GetName()));
@@ -566,6 +657,14 @@ using UnresolvedMemberRewriter =
 
 using UsingDeclRewriter = RewriterBase<clang::UsingDecl, clang::NamedDecl>;
 
+using DependentScopeDeclRefExprRewriter =
+    RewriterBase<clang::DependentScopeDeclRefExpr,
+                 clang::DependentScopeDeclRefExpr>;
+
+using CXXDependentScopeMemberExprRewriter =
+    RewriterBase<clang::CXXDependentScopeMemberExpr,
+                 clang::CXXDependentScopeMemberExpr>;
+
 }  // namespace
 
 static llvm::cl::extrahelp common_help(CommonOptionsParser::HelpMessage);
@@ -585,9 +684,9 @@ int main(int argc, const char* argv[]) {
   std::set<Replacement> replacements;
 
   auto in_blink_namespace =
-      decl(hasAncestor(namespaceDecl(anyOf(hasName("blink"), hasName("WTF")),
-                                     hasParent(translationUnitDecl()))),
-           unless(isExpansionInFileMatching(kGeneratedFileRegex)));
+      allOf(hasAncestor(namespaceDecl(anyOf(hasName("blink"), hasName("WTF")),

[Łukasz Anforowicz, 2016/08/17 22:45:09]

s/decl/allOf/ because DependentScopeDeclRefExpr and CXXDependentScopeMemberExpr
are not a "decl" (i.e. their matchers do not accept a matcher/filter limited to
decls only [like |in_blink_namespace| was before this change]).

+                                      hasParent(translationUnitDecl()))),
+            unless(isExpansionInFileMatching(kGeneratedFileRegex)));
 
   // Field, variable, and enum declarations ========
   // Given
@@ -835,6 +934,31 @@ int main(int argc, const char* argv[]) {
   UsingDeclRewriter using_decl_rewriter(&replacements);
   match_finder.addMatcher(using_decl_matcher, &using_decl_rewriter);
 
+  // Template-dependent decl lookup ========
+  // Given
+  //   template <typename T> void f() { T::foo(); }
+  // matches |T::foo|.
+  auto dependent_scope_decl_ref_expr_matcher =
+      expr(id("decl", dependentScopeDeclRefExpr(in_blink_namespace)));

[Łukasz Anforowicz, 2016/08/17 22:45:09]

One could argue that I should say "expr" instead of "decl" above... But
RewriterBase::run hardcodes "decl"...  Path of least resistance... :-)  One
alternative would be to have a trait for GetTargetName (old GetName) and for
GetSourceName) - this seems like too much trouble IMO.

+  DependentScopeDeclRefExprRewriter dependent_scope_decl_ref_expr_rewriter(
+      &replacements);
+  match_finder.addMatcher(dependent_scope_decl_ref_expr_matcher,
+                          &dependent_scope_decl_ref_expr_rewriter);
+
+  // Template-dependent member lookup ========
+  // Given
+  //   template <typename T>
+  //   class Foo {
+  //     void f() { T::foo(); }
+  //   };
+  // matches |T::foo|.
+  auto cxx_dependent_scope_member_expr_matcher =
+      expr(id("decl", cxxDependentScopeMemberExpr(in_blink_namespace)));
+  CXXDependentScopeMemberExprRewriter cxx_dependent_scope_member_expr_rewriter(
+      &replacements);
+  match_finder.addMatcher(cxx_dependent_scope_member_expr_matcher,
+                          &cxx_dependent_scope_member_expr_rewriter);
+
   std::unique_ptr<clang::tooling::FrontendActionFactory> factory =
       clang::tooling::newFrontendActionFactory(&match_finder);
   int result = tool.run(factory.get());