238 lines
12 KiB
Diff
238 lines
12 KiB
Diff
From cdf3e81ff49b200213d67d65558f2919222b60ab Mon Sep 17 00:00:00 2001
|
|
From: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
|
|
Date: Mon, 16 Dec 2019 11:39:11 +0000
|
|
Subject: [PATCH] BookmarkModelMerger: Move RemoteTreeNode declaration to
|
|
header.
|
|
|
|
This fixes the build with libstdc++ after commit 8f5dad93e58 ("Fix CHECK
|
|
failure due to untracked local nodes"):
|
|
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/stl_pair.h:215:11: error: field has incomplete type 'sync_bookmarks::BookmarkModelMerger::RemoteTreeNode'
|
|
_T2 second; /// @c second is a copy of the second object
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/ext/aligned_buffer.h:91:28: note: in instantiation of template class 'std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>' requested here
|
|
: std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:233:43: note: in instantiation of template class '__gnu_cxx::__aligned_buffer<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >' requested here
|
|
__gnu_cxx::__aligned_buffer<_Value> _M_storage;
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:264:39: note: in instantiation of template class 'std::__detail::_Hash_node_value_base<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >' requested here
|
|
struct _Hash_node<_Value, true> : _Hash_node_value_base<_Value>
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:2028:25: note: in instantiation of template class 'std::__detail::_Hash_node<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, true>' requested here
|
|
rebind_traits<typename __node_type::value_type>;
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable.h:184:15: note: in instantiation of template class 'std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, true> > >
|
|
' requested here
|
|
private __detail::_Hashtable_alloc<
|
|
^
|
|
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/unordered_map.h:105:18: note: in instantiation of template class 'std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, std::allocator<std::pair<con
|
|
st std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char> >, std::hash<std::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__deta
|
|
il::_Hashtable_traits<true, false, true> >' requested here
|
|
_Hashtable _M_h;
|
|
^
|
|
../../components/sync_bookmarks/bookmark_model_merger.h:146:22: note: in instantiation of template class 'std::unordered_map<std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode, std::hash<std::string>, std::equal_to<std::__cxx11::basic_string<char> >, std::allocator<std::pair<con
|
|
st std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> > >' requested here
|
|
const RemoteForest remote_forest_;
|
|
^
|
|
../../components/sync_bookmarks/bookmark_model_merger.h:53:9: note: forward declaration of 'sync_bookmarks::BookmarkModelMerger::RemoteTreeNode'
|
|
class RemoteTreeNode;
|
|
^
|
|
|
|
Essentially, the problem is that libstdc++'s std::unordered_map<T, U>
|
|
implementation requires both T and U to be fully declared. I raised the
|
|
problem in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92770, and GCC's
|
|
position is that we are relying on undefined behavior according to the C++
|
|
standard (https://eel.is/c++draft/requirements#res.on.functions-2.5).
|
|
|
|
Bug: 957519
|
|
Change-Id: Ife7e435e516932a795bfbe05b2c910c3272878f0
|
|
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1960156
|
|
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
|
|
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
|
|
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
|
|
Cr-Commit-Position: refs/heads/master@{#725070}
|
|
---
|
|
.../sync_bookmarks/bookmark_model_merger.cc | 89 +++++++------------
|
|
.../sync_bookmarks/bookmark_model_merger.h | 48 +++++++++-
|
|
2 files changed, 80 insertions(+), 57 deletions(-)
|
|
|
|
diff --git a/components/sync_bookmarks/bookmark_model_merger.cc b/components/sync_bookmarks/bookmark_model_merger.cc
|
|
index eae153eff95..579848ee664 100644
|
|
--- a/components/sync_bookmarks/bookmark_model_merger.cc
|
|
+++ b/components/sync_bookmarks/bookmark_model_merger.cc
|
|
@@ -5,7 +5,6 @@
|
|
#include "components/sync_bookmarks/bookmark_model_merger.h"
|
|
|
|
#include <algorithm>
|
|
-#include <memory>
|
|
#include <set>
|
|
#include <string>
|
|
#include <utility>
|
|
@@ -205,66 +204,44 @@ UpdatesPerParentId GroupValidUpdatesByParentId(
|
|
|
|
} // namespace
|
|
|
|
-class BookmarkModelMerger::RemoteTreeNode final {
|
|
- public:
|
|
- // Constructs a tree given |update| as root and recursively all descendants by
|
|
- // traversing |*updates_per_parent_id|. |update| and |updates_per_parent_id|
|
|
- // must not be null. All updates |*updates_per_parent_id| must represent valid
|
|
- // updates. Updates corresponding from descendant nodes are moved away from
|
|
- // |*updates_per_parent_id|.
|
|
- static RemoteTreeNode BuildTree(
|
|
- std::unique_ptr<syncer::UpdateResponseData> update,
|
|
- UpdatesPerParentId* updates_per_parent_id);
|
|
-
|
|
- ~RemoteTreeNode() = default;
|
|
-
|
|
- // Allow moves, useful during construction.
|
|
- RemoteTreeNode(RemoteTreeNode&&) = default;
|
|
- RemoteTreeNode& operator=(RemoteTreeNode&&) = default;
|
|
-
|
|
- const syncer::EntityData& entity() const { return *update_->entity; }
|
|
- int64_t response_version() const { return update_->response_version; }
|
|
-
|
|
- // Direct children nodes, sorted by ascending unique position. These are
|
|
- // guaranteed to be valid updates (e.g. IsValidBookmarkSpecifics()).
|
|
- const std::vector<RemoteTreeNode>& children() const { return children_; }
|
|
-
|
|
- // Recursively emplaces all GUIDs (this node and descendants) into
|
|
- // |*guid_to_remote_node_map|, which must not be null.
|
|
- void EmplaceSelfAndDescendantsByGUID(
|
|
- std::unordered_map<std::string, const RemoteTreeNode*>*
|
|
- guid_to_remote_node_map) const {
|
|
- DCHECK(guid_to_remote_node_map);
|
|
-
|
|
- const std::string& guid = entity().specifics.bookmark().guid();
|
|
- if (!guid.empty()) {
|
|
- DCHECK(base::IsValidGUID(guid));
|
|
-
|
|
- // Duplicate GUIDs have been sorted out before.
|
|
- bool success = guid_to_remote_node_map->emplace(guid, this).second;
|
|
- DCHECK(success);
|
|
- }
|
|
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode() = default;
|
|
|
|
- for (const RemoteTreeNode& child : children_) {
|
|
- child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
|
|
- }
|
|
- }
|
|
+BookmarkModelMerger::RemoteTreeNode::~RemoteTreeNode() = default;
|
|
+
|
|
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode(
|
|
+ BookmarkModelMerger::RemoteTreeNode&&) = default;
|
|
+BookmarkModelMerger::RemoteTreeNode& BookmarkModelMerger::RemoteTreeNode::
|
|
+operator=(BookmarkModelMerger::RemoteTreeNode&&) = default;
|
|
+
|
|
+void BookmarkModelMerger::RemoteTreeNode::EmplaceSelfAndDescendantsByGUID(
|
|
+ std::unordered_map<std::string, const RemoteTreeNode*>*
|
|
+ guid_to_remote_node_map) const {
|
|
+ DCHECK(guid_to_remote_node_map);
|
|
+
|
|
+ const std::string& guid = entity().specifics.bookmark().guid();
|
|
+ if (!guid.empty()) {
|
|
+ DCHECK(base::IsValidGUID(guid));
|
|
|
|
- private:
|
|
- static bool UniquePositionLessThan(const RemoteTreeNode& lhs,
|
|
- const RemoteTreeNode& rhs) {
|
|
- const syncer::UniquePosition a_pos =
|
|
- syncer::UniquePosition::FromProto(lhs.entity().unique_position);
|
|
- const syncer::UniquePosition b_pos =
|
|
- syncer::UniquePosition::FromProto(rhs.entity().unique_position);
|
|
- return a_pos.LessThan(b_pos);
|
|
+ // Duplicate GUIDs have been sorted out before.
|
|
+ bool success = guid_to_remote_node_map->emplace(guid, this).second;
|
|
+ DCHECK(success);
|
|
}
|
|
|
|
- RemoteTreeNode() = default;
|
|
+ for (const RemoteTreeNode& child : children_) {
|
|
+ child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
|
|
+ }
|
|
+}
|
|
|
|
- std::unique_ptr<syncer::UpdateResponseData> update_;
|
|
- std::vector<RemoteTreeNode> children_;
|
|
-};
|
|
+// static
|
|
+bool BookmarkModelMerger::RemoteTreeNode::UniquePositionLessThan(
|
|
+ const RemoteTreeNode& lhs,
|
|
+ const RemoteTreeNode& rhs) {
|
|
+ const syncer::UniquePosition a_pos =
|
|
+ syncer::UniquePosition::FromProto(lhs.entity().unique_position);
|
|
+ const syncer::UniquePosition b_pos =
|
|
+ syncer::UniquePosition::FromProto(rhs.entity().unique_position);
|
|
+ return a_pos.LessThan(b_pos);
|
|
+}
|
|
|
|
// static
|
|
BookmarkModelMerger::RemoteTreeNode
|
|
diff --git a/components/sync_bookmarks/bookmark_model_merger.h b/components/sync_bookmarks/bookmark_model_merger.h
|
|
index 9b592000dc5..bf0783ecf8e 100644
|
|
--- a/components/sync_bookmarks/bookmark_model_merger.h
|
|
+++ b/components/sync_bookmarks/bookmark_model_merger.h
|
|
@@ -5,6 +5,7 @@
|
|
#ifndef COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_MODEL_MERGER_H_
|
|
#define COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_MODEL_MERGER_H_
|
|
|
|
+#include <memory>
|
|
#include <string>
|
|
#include <unordered_map>
|
|
#include <vector>
|
|
@@ -50,7 +51,52 @@ class BookmarkModelMerger {
|
|
|
|
private:
|
|
// Internal representation of a remote tree, composed of nodes.
|
|
- class RemoteTreeNode;
|
|
+ class RemoteTreeNode final {
|
|
+ private:
|
|
+ using UpdatesPerParentId =
|
|
+ std::unordered_map<base::StringPiece,
|
|
+ syncer::UpdateResponseDataList,
|
|
+ base::StringPieceHash>;
|
|
+
|
|
+ public:
|
|
+ // Constructs a tree given |update| as root and recursively all descendants
|
|
+ // by traversing |*updates_per_parent_id|. |update| and
|
|
+ // |updates_per_parent_id| must not be null. All updates
|
|
+ // |*updates_per_parent_id| must represent valid updates. Updates
|
|
+ // corresponding from descendant nodes are moved away from
|
|
+ // |*updates_per_parent_id|.
|
|
+ static RemoteTreeNode BuildTree(
|
|
+ std::unique_ptr<syncer::UpdateResponseData> update,
|
|
+ UpdatesPerParentId* updates_per_parent_id);
|
|
+
|
|
+ ~RemoteTreeNode();
|
|
+
|
|
+ // Allow moves, useful during construction.
|
|
+ RemoteTreeNode(RemoteTreeNode&&);
|
|
+ RemoteTreeNode& operator=(RemoteTreeNode&&);
|
|
+
|
|
+ const syncer::EntityData& entity() const { return *update_->entity; }
|
|
+ int64_t response_version() const { return update_->response_version; }
|
|
+
|
|
+ // Direct children nodes, sorted by ascending unique position. These are
|
|
+ // guaranteed to be valid updates (e.g. IsValidBookmarkSpecifics()).
|
|
+ const std::vector<RemoteTreeNode>& children() const { return children_; }
|
|
+
|
|
+ // Recursively emplaces all GUIDs (this node and descendants) into
|
|
+ // |*guid_to_remote_node_map|, which must not be null.
|
|
+ void EmplaceSelfAndDescendantsByGUID(
|
|
+ std::unordered_map<std::string, const RemoteTreeNode*>*
|
|
+ guid_to_remote_node_map) const;
|
|
+
|
|
+ private:
|
|
+ static bool UniquePositionLessThan(const RemoteTreeNode& lhs,
|
|
+ const RemoteTreeNode& rhs);
|
|
+
|
|
+ RemoteTreeNode();
|
|
+
|
|
+ std::unique_ptr<syncer::UpdateResponseData> update_;
|
|
+ std::vector<RemoteTreeNode> children_;
|
|
+ };
|
|
|
|
// A forest composed of multiple trees where the root of each tree represents
|
|
// a permanent node, keyed by server-defined unique tag of the root.
|