Commit 380e485b authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot

Emit constexpr-initialized fields as candidates for exclusion (vars).

This CL is similar to r820840, but handles fields initialized not in
constexpr constructors, but instead in constexpr variables.  After this
CL, the rewriter will detect fields that are initialized with
non-nullptr value in an initializer of a constexpr variable.  Such
fields will be emitted as candidates for exclusion via --exclude-fields
cmdline parameter.

Bug: 1069567
Change-Id: I35a901276752b21b4b4826e8447381d5647f7d0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514763
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823736}
parent 8c151991
......@@ -485,6 +485,30 @@ const clang::FieldDecl* GetExplicitDecl(const clang::FieldDecl* field_decl) {
return field_decl;
}
// Given:
// template <typename T>
// class MyTemplate {
// T field; // This is an explicit field declaration.
// };
// void foo() {
// // This creates implicit template specialization for MyTemplate,
// // including an implicit |field| declaration.
// MyTemplate<int> v;
// v.field = 123;
// }
// and
// innerMatcher that will match the explicit |T field| declaration (but not
// necessarily the implicit template declarations),
// hasExplicitFieldDecl(innerMatcher) will match both explicit and implicit
// field declarations.
//
// For example, |member_expr_matcher| below will match |v.field| in the example
// above, even though the type of |v.field| is |int|, rather than |T| (matched
// by substTemplateTypeParmType()):
// auto explicit_field_decl_matcher =
// fieldDecl(hasType(substTemplateTypeParmType()));
// auto member_expr_matcher = memberExpr(member(fieldDecl(
// hasExplicitFieldDecl(explicit_field_decl_matcher))))
AST_MATCHER_P(clang::FieldDecl,
hasExplicitFieldDecl,
clang::ast_matchers::internal::Matcher<clang::FieldDecl>,
......@@ -700,6 +724,68 @@ AST_MATCHER_P(clang::QualType,
return false;
}
// forEachInitExprWithFieldDecl matches InitListExpr if it
// 1) evaluates to a RecordType
// 2) has a InitListExpr + FieldDecl pair that matches the submatcher args.
//
// forEachInitExprWithFieldDecl is based on and very similar to the builtin
// forEachArgumentWithParam matcher.
AST_MATCHER_P2(clang::InitListExpr,
forEachInitExprWithFieldDecl,
clang::ast_matchers::internal::Matcher<clang::Expr>,
init_expr_matcher,
clang::ast_matchers::internal::Matcher<clang::FieldDecl>,
field_decl_matcher) {
const clang::InitListExpr& init_list_expr = Node;
const clang::Type* type = init_list_expr.getType()
.getDesugaredType(Finder->getASTContext())
.getTypePtrOrNull();
if (!type)
return false;
const clang::CXXRecordDecl* record_decl = type->getAsCXXRecordDecl();
if (!record_decl)
return false;
bool is_matching = false;
clang::ast_matchers::internal::BoundNodesTreeBuilder result;
const std::vector<const clang::FieldDecl*> field_decls(
record_decl->field_begin(), record_decl->field_end());
for (unsigned i = 0; i < init_list_expr.getNumInits(); i++) {
const clang::Expr* expr = init_list_expr.getInit(i);
const clang::FieldDecl* field_decl = nullptr;
if (const clang::ImplicitValueInitExpr* implicit_value_init_expr =
clang::dyn_cast<clang::ImplicitValueInitExpr>(expr)) {
continue; // Do not match implicit value initializers.
} else if (const clang::DesignatedInitExpr* designated_init_expr =
clang::dyn_cast<clang::DesignatedInitExpr>(expr)) {
// Nested designators are unsupported by C++.
if (designated_init_expr->size() != 1)
break;
expr = designated_init_expr->getInit();
field_decl = designated_init_expr->getDesignator(0)->getField();
} else {
if (i >= field_decls.size())
break;
field_decl = field_decls[i];
}
clang::ast_matchers::internal::BoundNodesTreeBuilder field_matches(
*Builder);
if (field_decl_matcher.matches(*field_decl, Finder, &field_matches)) {
clang::ast_matchers::internal::BoundNodesTreeBuilder expr_matches(
field_matches);
if (init_expr_matcher.matches(*expr, Finder, &expr_matches)) {
result.addMatch(expr_matches);
is_matching = true;
}
}
}
*Builder = std::move(result);
return is_matching;
}
// Rewrites |SomeClass* field| (matched as "affectedFieldDecl") into
// |CheckedPtr<SomeClass> field| and for each file rewritten in such way adds an
// |#include "base/memory/checked_ptr.h"|.
......@@ -1064,16 +1150,30 @@ int main(int argc, const char* argv[]) {
&overlapping_field_decl_writer);
// Matches fields initialized with a non-nullptr value in a constexpr
// constructor. See also the testcase in tests/gen-constexpr-ctor-test.cc.
auto constexpr_ctor_initialized_field_matcher = cxxConstructorDecl(allOf(
isConstexpr(),
forEachConstructorInitializer(allOf(
forField(field_decl_matcher), withInitializer(unless(ignoringImplicit(
cxxNullPtrLiteralExpr())))))));
FilteredExprWriter constexpr_ctor_initialized_field_writer(
&output_helper, "constexpr-ctor-initializer");
match_finder.addMatcher(constexpr_ctor_initialized_field_matcher,
&constexpr_ctor_initialized_field_writer);
// constructor. See also the testcase in tests/gen-constexpr-test.cc.
auto non_nullptr_expr_matcher =
expr(unless(ignoringImplicit(cxxNullPtrLiteralExpr())));
auto constexpr_ctor_field_initializer_matcher = cxxConstructorDecl(
allOf(isConstexpr(), forEachConstructorInitializer(allOf(
forField(field_decl_matcher),
withInitializer(non_nullptr_expr_matcher)))));
FilteredExprWriter constexpr_ctor_field_initializer_writer(
&output_helper, "constexpr-ctor-field-initializer");
match_finder.addMatcher(constexpr_ctor_field_initializer_matcher,
&constexpr_ctor_field_initializer_writer);
// Matches constexpr initializer list expressions that initialize a rewritable
// field with a non-nullptr value. For more details and rationale see the
// testcases in tests/gen-constexpr-test.cc.
auto constexpr_var_initializer_matcher = varDecl(
allOf(isConstexpr(),
hasInitializer(findAll(initListExpr(forEachInitExprWithFieldDecl(
non_nullptr_expr_matcher,
hasExplicitFieldDecl(field_decl_matcher)))))));
FilteredExprWriter constexpr_var_initializer_writer(
&output_helper, "constexpr-var-initializer");
match_finder.addMatcher(constexpr_var_initializer_matcher,
&constexpr_var_initializer_writer);
// See the doc comment for the isInMacroLocation matcher
// and the testcases in tests/gen-macro-test.cc.
......
......@@ -4,6 +4,7 @@
#include <stdint.h> // for uintptr_t
#include <string>
#include <tuple> // for std::tie
#include <utility> // for std::swap
......
......@@ -4,6 +4,7 @@
#include <stdint.h> // for uintptr_t
#include <string>
#include <tuple> // for std::tie
#include <utility> // for std::swap
......
==== BEGIN EDITS ====
include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-ctor-actual.cc:::-1:::-1:::base/memory/checked_ptr.h
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-ctor-actual.cc:::1045:::5:::CheckedPtr<int>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-ctor-actual.cc:::1153:::5:::CheckedPtr<int>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-ctor-actual.cc:::1271:::5:::CheckedPtr<int>
==== END EDITS ====
==== BEGIN FIELD FILTERS ====
Foo::ptr2_ # constexpr-ctor-initializer
Foo::ptr_ # constexpr-ctor-initializer
==== END FIELD FILTERS ====
==== BEGIN EDITS ====
include-user-header:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::-1:::-1:::base/memory/checked_ptr.h
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::1094:::5:::CheckedPtr<int>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::1202:::5:::CheckedPtr<int>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::1320:::5:::CheckedPtr<int>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2030:::16:::CheckedPtr<const char16_t>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2439:::16:::CheckedPtr<const char16_t>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2862:::12:::CheckedPtr<const char>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2893:::12:::CheckedPtr<const char>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2922:::12:::CheckedPtr<const char>
r:::/usr/local/google/home/lukasza/src/chromium4/src/tools/clang/rewrite_raw_ptr_fields/tests/gen-constexpr-actual.cc:::2944:::12:::CheckedPtr<const char>
==== END EDITS ====
==== BEGIN FIELD FILTERS ====
constexpr_variable_designated_initializers::foo()::(anonymous struct)::str2 # constexpr-var-initializer
constexpr_variable_designated_initializers::foo()::(anonymous struct)::str3 # constexpr-var-initializer
constexpr_variable_initializer::foo()::(anonymous struct)::str # constexpr-var-initializer
field_initializer_in_constexpr_ctor::Foo::ptr2_ # constexpr-ctor-field-initializer
field_initializer_in_constexpr_ctor::Foo::ptr_ # constexpr-ctor-field-initializer
==== END FIELD FILTERS ====
......@@ -12,6 +12,8 @@
//
// To run the test use tools/clang/rewrite_raw_ptr_fields/tests/run_all_tests.py
namespace field_initializer_in_constexpr_ctor {
class Foo {
public:
constexpr explicit Foo(int* ptr) : ptr_(ptr), ptr2_(ptr), null_(nullptr) {}
......@@ -30,3 +32,73 @@ class Foo {
// need to be skipped.
int* null_;
};
} // namespace field_initializer_in_constexpr_ctor
namespace constexpr_variable_initializer {
void foo() {
// The |str| field below should be emitted as a candidate for the
// --field-filter-file using the "constexpr-var-initializer" rule.
//
// This example is based on UtfOffsetTest.Utf8OffsetFromUtf16Offset in
// //ui/base/ime/utf_offset_unittest.cc
//
// Note that in this example, kTestCases does not have a global scope and
// therefore won't be covered by the "global-scope" heuristic. Similarly,
// there is no explicit constexpr constructor here, so the example won't be
// covered by the "constexpr-ctor-field-initializer" heuristic.
constexpr struct {
const char16_t* str;
int offset;
} kTestCases[] = {
{u"ab", 0},
{u"ab", 1},
{u"ab", 2},
};
}
} // namespace constexpr_variable_initializer
namespace constexpr_variable_uninitialized_field {
void foo() {
// The |str| field is not covered by the initializers below and therefore
// should not be emitted as a --field-filter-file candidate.
constexpr struct {
int i1;
const char16_t* str;
} kTestCases[] = {
{0},
{1},
{2},
};
}
} // namespace constexpr_variable_uninitialized_field
namespace constexpr_variable_designated_initializers {
void foo() {
// The |str2| and |str3| fields below (but not |str_uncovered|) are
// initialized by a designated initializer and should be emitted as a
// --field-filter-file candidate.
constexpr struct {
int i1;
const char* str_uncovered;
const char* str_nullptr;
const char* str2;
const char* str3;
} kTestCases[] = {
// Test to verify that all designated initializers are covered.
{.str2 = "blah", .str3 = "foo"},
// Tests to verify that nullptr initialization doesn't exclude a field
// (since BackupRefPtr has a constexpr ctor for nullptr_t).
{.str_nullptr = nullptr},
// Tests to verify that we don't accidentally cover |str_uncovered|.
{1},
{.i1 = 2},
};
}
} // namespace constexpr_variable_designated_initializers
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