Commit 14c85429 authored by dcheng's avatar dcheng Committed by Commit bot

rewrite_to_chrome_style: skip nodes in synthesized functions.

Trickily enough, there were two matchers triggering this rewrite:
- the reference to the field being initialized in the synthesized copy
  constructor
- the reference to the field being copied in the synthesized copy
  constructor

BUG=581871

Review URL: https://codereview.chromium.org/1639133004

Cr-Commit-Position: refs/heads/master@{#372246}
parent 98ce11e5
...@@ -53,6 +53,12 @@ AST_MATCHER(clang::FunctionDecl, isOverloadedOperator) { ...@@ -53,6 +53,12 @@ AST_MATCHER(clang::FunctionDecl, isOverloadedOperator) {
return Node.isOverloadedOperator(); return Node.isOverloadedOperator();
} }
// This is available in newer clang revisions... but alas, Chrome has not rolled
// that far yet.
AST_MATCHER(clang::FunctionDecl, isDefaulted) {
return Node.isDefaulted();
}
const char kBlinkFieldPrefix[] = "m_"; const char kBlinkFieldPrefix[] = "m_";
const char kBlinkStaticMemberPrefix[] = "s_"; const char kBlinkStaticMemberPrefix[] = "s_";
...@@ -230,6 +236,7 @@ struct TargetNodeTraits<clang::CXXCtorInitializer> { ...@@ -230,6 +236,7 @@ struct TargetNodeTraits<clang::CXXCtorInitializer> {
static const char kName[]; static const char kName[];
static clang::CharSourceRange GetRange( static clang::CharSourceRange GetRange(
const clang::CXXCtorInitializer& init) { const clang::CXXCtorInitializer& init) {
assert(init.isWritten());
return clang::CharSourceRange::getTokenRange(init.getSourceLocation()); return clang::CharSourceRange::getTokenRange(init.getSourceLocation());
} }
}; };
...@@ -415,7 +422,16 @@ int main(int argc, const char* argv[]) { ...@@ -415,7 +422,16 @@ int main(int argc, const char* argv[]) {
// ... // ...
// } // }
// matches |x| in if (x). // matches |x| in if (x).
auto member_matcher = id("expr", memberExpr(member(field_decl_matcher))); auto member_matcher = id(
"expr",
memberExpr(
member(field_decl_matcher),
// Needed to avoid matching member references in functions (which will
// be an ancestor of the member reference) synthesized by the
// compiler, such as a synthesized copy constructor.
// This skips explicitly defaulted functions as well, but that's OK:
// there's nothing interesting to rewrite in those either.
unless(hasAncestor(functionDecl(isDefaulted())))));
auto decl_ref_matcher = id("expr", declRefExpr(to(var_decl_matcher))); auto decl_ref_matcher = id("expr", declRefExpr(to(var_decl_matcher)));
MemberRewriter member_rewriter(&replacements); MemberRewriter member_rewriter(&replacements);
...@@ -531,7 +547,8 @@ int main(int argc, const char* argv[]) { ...@@ -531,7 +547,8 @@ int main(int argc, const char* argv[]) {
// matches each initializer in the constructor for S. // matches each initializer in the constructor for S.
auto constructor_initializer_matcher = auto constructor_initializer_matcher =
cxxConstructorDecl(forEachConstructorInitializer( cxxConstructorDecl(forEachConstructorInitializer(
id("initializer", cxxCtorInitializer(forField(field_decl_matcher))))); id("initializer",
cxxCtorInitializer(forField(field_decl_matcher), isWritten()))));
ConstructorInitializerRewriter constructor_initializer_rewriter( ConstructorInitializerRewriter constructor_initializer_rewriter(
&replacements); &replacements);
......
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
namespace blink { namespace blink {
// Note: do not add any copy or move constructors to this class: doing so will
// break test coverage that we don't clobber the class name by trying to emit
// replacements for synthesized functions.
class C { class C {
public: public:
// Make sure initializers are updated to use the new names. // Make sure initializers are updated to use the new names.
...@@ -50,4 +53,8 @@ union U { ...@@ -50,4 +53,8 @@ union U {
void F() { void F() {
// Test that references to a static field are correctly rewritten. // Test that references to a static field are correctly rewritten.
blink::C::instance_count_++; blink::C::instance_count_++;
// Force instantiation of a copy constructor for blink::C to make sure field
// initializers for synthesized functions don't cause weird rewrites.
blink::C c;
blink::C c2 = c;
} }
...@@ -4,6 +4,9 @@ ...@@ -4,6 +4,9 @@
namespace blink { namespace blink {
// Note: do not add any copy or move constructors to this class: doing so will
// break test coverage that we don't clobber the class name by trying to emit
// replacements for synthesized functions.
class C { class C {
public: public:
// Make sure initializers are updated to use the new names. // Make sure initializers are updated to use the new names.
...@@ -50,4 +53,8 @@ union U { ...@@ -50,4 +53,8 @@ union U {
void F() { void F() {
// Test that references to a static field are correctly rewritten. // Test that references to a static field are correctly rewritten.
blink::C::instanceCount++; blink::C::instanceCount++;
// Force instantiation of a copy constructor for blink::C to make sure field
// initializers for synthesized functions don't cause weird rewrites.
blink::C c;
blink::C c2 = c;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment