Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Workaround a release blocker after libc++ change #43091

Merged
auto-submit merged 3 commits into from
Jun 22, 2023
Merged

Workaround a release blocker after libc++ change #43091

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09e5159
Workaround a release blocker after libc++ change
chingjun Jun 22, 2023
34410f6
Update comments
chingjun Jun 22, 2023
a788a9c
Update comments
chingjun Jun 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions shell/platform/common/client_wrapper/include/flutter/encodable_value.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,13 @@ class EncodableValue : public internal::EncodableValueVariant {
}
return std::get<int64_t>(*this);
}

// Explicitly provide operator<, delegating to std::variant's operator<.
// There are issues with with the way the standard library-provided
// < and <=> comparisons interact with classes derived from variant.
friend bool operator<(const EncodableValue& lhs, const EncodableValue& rhs) {
return static_cast<const super&>(lhs) < static_cast<const super&>(rhs);
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not following this code. What does an explicit cast to a super reference do? This needs an explanatory comment.

Copy link
Contributor Author

@chingjun chingjun Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From internal change:

This code says "define < comparison of encodable values as the < comparison of the underlying variant". (I've added a comment saying this)

The cast is needed because lhs < rhs would just recursively call the operator< we're trying to define here. (I wouldn't normally add a comment to this, feel free to suggest one)

The high level idea is: expose an operator< directly on EncodableType so that vector<EncodableType> < vector<EncodableType> will delegate to that. Currently such comparisons fail to compile, for reasons that are pretty obscure.

My best understanding is that variant's/vector's operator<=> do not work properly when you inherit from variant/vector, and this is not a bug in libc++ but is specified in C++20. (I would further guess: this is not specifically intended, but an interaction of many factors and backwards-compat for code that inherits from stdlib classes is not a high priority. operator<=> is an outlier in that it breaks more existing code than most language changes).

In any case, most of this is out of our hands: the new compiler rejects this code, it appears to be correct in doing so, I don't have a simple intuitive explanation why. Happy to add whatever comments you like though, please suggest some text.

}
};

} // namespace flutter
Expand Down