You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
chromium/chromium-80-gcc-incomplete-...

230 lines
11 KiB

5 years ago
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}
---
diff --git a/components/sync_bookmarks/bookmark_model_merger.cc b/components/sync_bookmarks/bookmark_model_merger.cc
index eae153ef..579848e 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 @@
} // 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);
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode() = default;
- ~RemoteTreeNode() = default;
+BookmarkModelMerger::RemoteTreeNode::~RemoteTreeNode() = default;
- // Allow moves, useful during construction.
- RemoteTreeNode(RemoteTreeNode&&) = default;
- RemoteTreeNode& operator=(RemoteTreeNode&&) = default;
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode(
+ BookmarkModelMerger::RemoteTreeNode&&) = default;
+BookmarkModelMerger::RemoteTreeNode& BookmarkModelMerger::RemoteTreeNode::
+operator=(BookmarkModelMerger::RemoteTreeNode&&) = default;
- const syncer::EntityData& entity() const { return *update_->entity; }
- int64_t response_version() const { return update_->response_version; }
+void BookmarkModelMerger::RemoteTreeNode::EmplaceSelfAndDescendantsByGUID(
+ std::unordered_map<std::string, const RemoteTreeNode*>*
+ guid_to_remote_node_map) const {
+ DCHECK(guid_to_remote_node_map);
- // 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_; }
+ const std::string& guid = entity().specifics.bookmark().guid();
+ if (!guid.empty()) {
+ DCHECK(base::IsValidGUID(guid));
- // 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);
- }
-
- for (const RemoteTreeNode& child : children_) {
- child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
- }
+ // Duplicate GUIDs have been sorted out before.
+ bool success = guid_to_remote_node_map->emplace(guid, this).second;
+ DCHECK(success);
}
- 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);
+ for (const RemoteTreeNode& child : children_) {
+ child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
}
+}
- RemoteTreeNode() = default;
-
- 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 9b59200..bf0783ec 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 @@
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.