Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(203)

Unified Diff: tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp

Issue 2246263002: Skip parameter replacements where old text != old name. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Self-review. Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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..9679c9ff5d80480c4e56aebd53648424f53ffbbb 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
@@ -342,8 +342,21 @@ bool GetNameForDecl(const clang::VarDecl& decl,
StringRef original_name = decl.getName();
// Nothing to do for unnamed parameters.
- if (clang::isa<clang::ParmVarDecl>(decl) && original_name.empty())
- return false;
+ if (clang::isa<clang::ParmVarDecl>(decl)) {
+ if (original_name.empty())
+ return false;
+
+ // Check if |decl| really covers text of |original_name|. This might not be
+ // true when specializing a templatized class that had an unnamed parameter
+ // in one of method declarations.
dcheng 2016/08/25 04:08:51 Let's file an upstream bug for this with the test
Łukasz Anforowicz 2016/08/25 23:32:33 Done: https://llvm.org/bugs/show_bug.cgi?id=29145
+ clang::SourceLocation loc = decl.getLocation();
+ clang::CharSourceRange range = clang::CharSourceRange::getTokenRange(loc);
+ StringRef old_text = clang::Lexer::getSourceText(
+ range, context.getSourceManager(), context.getLangOpts());
+ if (!loc.isMacroID() && old_text != original_name) {
dcheng 2016/08/25 04:08:52 Add a comment why we want to ignore SourceLocation
Łukasz Anforowicz 2016/08/25 23:32:33 I've started using spelling location in the latest
+ 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_`

Powered by Google App Engine
This is Rietveld 408576698