rewrite_to_chrome_style: Make is-const decisions more consistent

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 373 matching lines @@
   // Recurse on children. If they are all const (or are uses of template
   // input) then the statement can be considered const. For whatever reason the
   // below checks can give different-and-less-consistent responses if we call
   // them on a complex expression than if we call them on the most primitive
   // pieces (some pieces would say false but the whole thing says true).
   for (auto* child : expr->children()) {
     if (!CanBeEvaluatedAtCompileTime(child, context))
       return false;
   }
 
-  // If the expression is a template input then its coming at compile time so
-  // we consider it const. And we can't check isEvaluatable() in this case as
-  // it will do bad things/crash.
-  if (expr->isInstantiationDependent())
-    return true;
+  // If the expression depends on template input, we can not call
+  // isEvaluatable() on it as it will do bad things/crash.
+  if (!expr->isInstantiationDependent()) {
+    // If the expression can be evaluated at compile time, then it should have a
+    // kFoo style name. Otherwise, not.
+    return expr->isEvaluatable(context);
+  }
 
-  // If the expression can be evaluated at compile time, then it should have a
-  // kFoo style name. Otherwise, not.
-  return expr->isEvaluatable(context);
+  // We do our best to figure out special cases as we come across them here, for
+  // template dependent situations. Some cases in code are only considered
+  // instantiation dependent for some template instantiations! Which is
+  // terrible! So most importantly we try to match isEvaluatable in those cases.
+  switch (expr->getStmtClass()) {
+    case clang::Stmt::CXXThisExprClass:
+      return false;
+    case clang::Stmt::DeclRefExprClass: {
+      auto* declref = clang::dyn_cast<clang::DeclRefExpr>(expr);
+      auto* decl = declref->getDecl();
+      if (clang::dyn_cast<clang::VarDecl>(decl))

[dcheng, 2017/01/11 17:03:12]

Nit: use isa... except I guess the followup CL changes this to use the result of
the dyn_cast, so never mind =)

+        return false;
+      break;
+    }
+
+    default:
+      break;
+  }
+
+  // Otherwise, we consider depending on template parameters to not interfere
+  // with being const.. with exceptions hopefully covered above.
+  return true;
 }
 
 bool IsProbablyConst(const clang::VarDecl& decl,
                      const clang::ASTContext& context) {
   clang::QualType type = decl.getType();
   if (!type.isConstQualified())
     return false;
 
   if (type.isVolatileQualified())
     return false;
@@ skipping 1074 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;
 }