You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "PragmaTwice (via GitHub)" <gi...@apache.org> on 2023/04/18 03:23:39 UTC
[GitHub] [arrow] PragmaTwice commented on a diff in pull request #34002: GH-34001: [C++] Add more rvalue overload for Status::operator&
PragmaTwice commented on code in PR #34002:
URL: https://github.com/apache/arrow/pull/34002#discussion_r1169448208
##########
cpp/src/arrow/status.h:
##########
@@ -157,8 +157,10 @@ class ARROW_EXPORT [[nodiscard]] Status : public util::EqualityComparable<Status
inline bool Equals(const Status& s) const;
// AND the statuses.
- inline Status operator&(const Status& s) const noexcept;
- inline Status operator&(Status&& s) const noexcept;
+ inline Status operator&(const Status& s) const& noexcept;
+ inline Status operator&(const Status& s) && noexcept;
+ inline Status operator&(Status&& s) const& noexcept;
+ inline Status operator&(Status&& s) && noexcept;
Review Comment:
I do not understand what you mean by the "simpler" that you mentioned.
It should be like this in c++23 or later:
```c++
template <typename Self, typename T, std::enable_if_t<std::is_same_v<std::remove_cvref_t<T>, Status>, int> = 0>
Status operator&(this Self&& self, T&& other) {
if (ok()) return std::forward<T>(other);
return std::forward<Self>(self);
}
```
(hand-written, you can check it by compiler and lang-server)
##########
cpp/src/arrow/status.h:
##########
@@ -157,8 +157,10 @@ class ARROW_EXPORT [[nodiscard]] Status : public util::EqualityComparable<Status
inline bool Equals(const Status& s) const;
// AND the statuses.
- inline Status operator&(const Status& s) const noexcept;
- inline Status operator&(Status&& s) const noexcept;
+ inline Status operator&(const Status& s) const& noexcept;
+ inline Status operator&(const Status& s) && noexcept;
+ inline Status operator&(Status&& s) const& noexcept;
+ inline Status operator&(Status&& s) && noexcept;
Review Comment:
I do not understand what you mean by the "simpler" that you mentioned.
It should be like this in c++23 or later:
```c++
template <typename Self, typename T,
std::enable_if_t<std::is_same_v<std::remove_cvref_t<T>, Status>, int> = 0>
Status operator&(this Self&& self, T&& other) {
if (ok()) return std::forward<T>(other);
return std::forward<Self>(self);
}
```
(hand-written, you can check it by compiler and lang-server)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org