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

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: Detecting buggy ParmVarDecls without looking at the source text. 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
« no previous file with comments | « docs/clang_tool_refactoring.md ('k') | tools/clang/rewrite_to_chrome_style/tests/template-expected.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 f651ba7ca6624076d09c3bb57971667535356895..283dc448b35532297c86ef9089a55f4c6995c2ca 100644
--- a/tools/clang/rewrite_to_chrome_style/RewriteToChromeStyle.cpp
+++ b/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())
- return false;
+ if (clang::isa<clang::ParmVarDecl>(decl)) {
+ 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);
« no previous file with comments | « docs/clang_tool_refactoring.md ('k') | tools/clang/rewrite_to_chrome_style/tests/template-expected.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698