Skip to content

Commit 4f03835

Browse files
Googlera-maurice
authored andcommitted
LSC: Change for loops to use const refs, per performance-for-range-copy
clang-tidy. This CL optimizes C++11 range-based for loops where the variable is copied in each iteration but it would suffice to obtain it by const reference. This is only applied to loop variables of types that are expensive to copy which means they are not trivially copyable or have a non-trivial copy constructor or destructor. To ensure that it is safe to replace the copy with a const reference the following heuristic is employed: The loop variable is const qualified. The loop variable is not const, but only const methods or operators are invoked on it, or it is used as const reference or value argument in onstructors or function calls. See go/lsc-performance-for-range-copy for details. Applied ClangTidy fixes to google3 paths matching '[fgh].*'. The changelist was automatically generated using the apply_fixes.sh script: devtools/cymbal/clang_tidy/apply_fixes.sh --checks=-*,performance-for-range-copy --create_changelist --files_to_process=[fgh].* Track: canary Findings source: /placer/prod/home/devtools-cxx-clang-tidy/canary/latest/clang_tidy-?????-of-????? Subcategories: performance-for-range-copy WARNING: Automatically generated changes should be carefully inspected. For most ClangTidy warnings multiple alternative fixes are possible and only one of them is suggested. There's no guarantee that it will be the best one (or even a right fix). The amount of reviewers' attention to automated changes should be at least the same as for manually written code. Tested: TAP --sample ran all affected tests and none failed http://test/OCL:311929150:BASE:311925626:1589687570441:53335266 PiperOrigin-RevId: 312080641
1 parent 4f94fbf commit 4f03835

File tree

6 files changed

+10
-10
lines changed

6 files changed

+10
-10
lines changed

database/src/desktop/core/repo.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ void Repo::PostEvents(const std::vector<Event>& events) {
479479
static std::map<Path, Variant> VariantToPathMap(const Variant& data) {
480480
std::map<Path, Variant> path_map;
481481
if (data.is_map()) {
482-
for (std::pair<Variant, Variant> key_value : data.map()) {
482+
for (const auto& key_value : data.map()) {
483483
Variant key_string_variant;
484484
const char* key;
485485
if (key_value.first.is_string()) {
@@ -539,7 +539,7 @@ void Repo::OnServerInfoUpdate(const std::string& key, const Variant& value) {
539539
void Repo::OnServerInfoUpdate(const std::map<Variant, Variant>& updates) {
540540
SAFE_REFERENCE_RETURN_VOID_IF_INVALID(ThisRefLock, lock, safe_this_);
541541

542-
for (const std::pair<Variant, Variant> element : updates) {
542+
for (const auto& element : updates) {
543543
const Variant& key = element.first;
544544
const Variant& value = element.second;
545545
UpdateInfo(key.AsString().mutable_string(), value);

database/src/desktop/core/sync_point.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ std::vector<Event> SyncPoint::AddEventRegistration(
9999
std::set<std::string> all_children;
100100
const Variant* value = GetVariantValue(&view->GetLocalCache());
101101
if (value->is_map()) {
102-
for (auto key_value_pair : value->map()) {
102+
for (const auto& key_value_pair : value->map()) {
103103
all_children.insert(key_value_pair.first.string_value());
104104
}
105105
}

database/src/desktop/core/sync_tree.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ std::vector<Event> SyncTree::RemoveEventRegistration(
638638
Tree<SyncPoint>* current_tree = &sync_point_tree_;
639639
bool covered = current_tree->value().has_value() &&
640640
current_tree->value()->HasCompleteView();
641-
for (std::string directory : query_spec.path.GetDirectories()) {
641+
for (const std::string& directory : query_spec.path.GetDirectories()) {
642642
current_tree = current_tree->GetChild(directory);
643643
covered = covered || (current_tree->value().has_value() &&
644644
current_tree->value()->HasCompleteView());
@@ -680,7 +680,7 @@ std::vector<Event> SyncTree::RemoveEventRegistration(
680680
listen_provider_->StopListening(QuerySpecForListening(query_spec),
681681
Tag());
682682
} else {
683-
for (QuerySpec query_to_remove : removed) {
683+
for (const QuerySpec& query_to_remove : removed) {
684684
Tag tag = TagForQuerySpec(query_to_remove);
685685
FIREBASE_DEV_ASSERT(tag.has_value());
686686
listen_provider_->StopListening(

database/src/desktop/core/tracked_query_manager.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ TrackedQueryManager::TrackedQueryManager(
9191
// Populate our cache from the storage layer.
9292
std::vector<TrackedQuery> tracked_queries =
9393
storage_engine_->LoadTrackedQueries();
94-
for (TrackedQuery query : tracked_queries) {
94+
for (const TrackedQuery& query : tracked_queries) {
9595
next_query_id_ = std::max(query.query_id + 1, next_query_id_);
9696
CacheTrackedQuery(query);
9797
}
@@ -161,8 +161,8 @@ void TrackedQueryManager::SetQueryCompleteIfExists(
161161
void TrackedQueryManager::SetQueriesComplete(const Path& path) {
162162
tracked_query_tree_.CallOnEach(
163163
path, [this](const Path& path, TrackedQueryMap& tracked_queries) {
164-
for (auto key_value : tracked_queries) {
165-
TrackedQuery& tracked_query = key_value.second;
164+
for (const auto& key_value : tracked_queries) {
165+
const TrackedQuery& tracked_query = key_value.second;
166166
if (!tracked_query.complete) {
167167
TrackedQuery updated_tracked_query = tracked_query;
168168
updated_tracked_query.complete = TrackedQuery::kComplete;

database/src/desktop/persistence/persistence_manager.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ CacheNode PersistenceManager::ServerCache(const QuerySpec& query_spec) {
111111
storage_engine_->ServerCache(query_spec.path);
112112
if (found_tracked_keys) {
113113
Variant filtered_node = Variant::EmptyMap();
114-
for (std::string key : tracked_keys) {
114+
for (const std::string& key : tracked_keys) {
115115
VariantUpdateChild(&filtered_node, key,
116116
VariantGetChild(&server_cache_node, key));
117117
}

database/src/desktop/view/view_processor.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -667,7 +667,7 @@ ViewCache ViewProcessor::AckUserWrite(const ViewCache& view_cache,
667667
// in our cache as a merge.
668668
CompoundWrite changed_children = CompoundWrite::EmptyWrite();
669669
if (server_cache.variant().is_map()) {
670-
for (auto key_value : server_cache.variant().map()) {
670+
for (const auto& key_value : server_cache.variant().map()) {
671671
CompoundWrite temp = changed_children.AddWrite(
672672
key_value.first.AsString().mutable_string(), key_value.second);
673673
changed_children = temp;

0 commit comments

Comments
 (0)