You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/21 20:45:44 UTC
[GitHub] [arrow] bkietz opened a new pull request #8735: ARROW-6781: [C++] Improve and consoldiate CHECK macros
bkietz opened a new pull request #8735:
URL: https://github.com/apache/arrow/pull/8735
Adds more descriptive failures for comparison CHECKs.
```c++
ARROW_CHECK(1 == 2) << "fails and prints operands";
// ../src/arrow/util/logging_test.cc:101: Check failed: 1 == 2
// left: 1
// right: 2
// fails and prints operands
ARROW_CHECK(std::string("hello") == "hello")
<< "differently typed operands are fine; they just need to "
<< "support the relevant comparision";
ARROW_CHECK(std::vector<int>{1, 2} == std::vector<int>{})
<< "vector isn't printable - no extra printing";
ARROW_CHECK_OK(Status::OK()) << "Status Checks can be streamed into";
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530348934
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -86,56 +67,40 @@ enum class ArrowLogLevel : int {
#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
-#ifdef NDEBUG
-#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
+#define ARROW_LOG_FAILED_STATUS(status, ...) \
+ ARROW_LOG(FATAL) << " Operation failed: " << ARROW_STRINGIFY(__VA_ARGS__) << "\n" \
+ << " Bad status: " << _s.ToString()
+
+// If the status is bad, CHECK immediately, appending the status to the logged
+// message.
+#define ARROW_CHECK_OK(...) \
+ for (::arrow::Status _s = ::arrow::internal::GenericToStatus((__VA_ARGS__)); \
Review comment:
The last tokens of a `do... while` loop are the loop condition, which precludes providing a stream for extra error messages.
`for` supports a scoped decl, which `while` doesn't. If we wanted to avoid `for`, we could introduce a variable as in `ARROW_ASSIGN_OR_RAISE`. After some macro expansion that'd look like:
```c++
Status _s_56 = Thing();
if (ARROW_PREDICT_FALSE(!_s_56.ok()))
ARROW_LOG(FATAL) << _s.message() << "Extra messages";
```
It seems more desirable to keep things scoped with a `for`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530353939
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -22,22 +22,21 @@
// The LLVM IR code doesn't have an NDEBUG mode. And, it shouldn't include references to
// streams or stdc++. So, making the DCHECK calls void in that case.
-#define ARROW_IGNORE_EXPR(expr) ((void)(expr))
-
-#define DCHECK(condition) ARROW_IGNORE_EXPR(condition)
-#define DCHECK_OK(status) ARROW_IGNORE_EXPR(status)
-#define DCHECK_EQ(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_NE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LT(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GT(val1, val2) ARROW_IGNORE_EXPR(val1)
+#define DCHECK(condition) ARROW_UNUSED(condition)
+#define DCHECK_OK(status) ARROW_UNUSED(status)
+#define DCHECK_EQ(val1, val2) ARROW_UNUSED(val1 == val2)
+#define DCHECK_NE(val1, val2) ARROW_UNUSED(val1 != val2)
+#define DCHECK_LE(val1, val2) ARROW_UNUSED(val1 <= val2)
+#define DCHECK_LT(val1, val2) ARROW_UNUSED(val1 < val2)
+#define DCHECK_GE(val1, val2) ARROW_UNUSED(val1 >= val2)
+#define DCHECK_GT(val1, val2) ARROW_UNUSED(val1 > val2)
Review comment:
Ah, I see. In that case I should prefix `if (false)` so the entire expr can be dropped
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#issuecomment-754109097
closing for now
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530187025
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -86,56 +67,40 @@ enum class ArrowLogLevel : int {
#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
-#ifdef NDEBUG
-#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
+#define ARROW_LOG_FAILED_STATUS(status, ...) \
+ ARROW_LOG(FATAL) << " Operation failed: " << ARROW_STRINGIFY(__VA_ARGS__) << "\n" \
+ << " Bad status: " << _s.ToString()
+
+// If the status is bad, CHECK immediately, appending the status to the logged
+// message.
+#define ARROW_CHECK_OK(...) \
+ for (::arrow::Status _s = ::arrow::internal::GenericToStatus((__VA_ARGS__)); \
Review comment:
Ok, but I mean compared to a regular `while` or `do...while`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #8735: ARROW-6781: [C++] Improve and consoldiate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#issuecomment-732160454
This PR needs rebasing now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530186803
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -22,22 +22,21 @@
// The LLVM IR code doesn't have an NDEBUG mode. And, it shouldn't include references to
// streams or stdc++. So, making the DCHECK calls void in that case.
-#define ARROW_IGNORE_EXPR(expr) ((void)(expr))
-
-#define DCHECK(condition) ARROW_IGNORE_EXPR(condition)
-#define DCHECK_OK(status) ARROW_IGNORE_EXPR(status)
-#define DCHECK_EQ(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_NE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LT(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GT(val1, val2) ARROW_IGNORE_EXPR(val1)
+#define DCHECK(condition) ARROW_UNUSED(condition)
+#define DCHECK_OK(status) ARROW_UNUSED(status)
+#define DCHECK_EQ(val1, val2) ARROW_UNUSED(val1 == val2)
+#define DCHECK_NE(val1, val2) ARROW_UNUSED(val1 != val2)
+#define DCHECK_LE(val1, val2) ARROW_UNUSED(val1 <= val2)
+#define DCHECK_LT(val1, val2) ARROW_UNUSED(val1 < val2)
+#define DCHECK_GE(val1, val2) ARROW_UNUSED(val1 >= val2)
+#define DCHECK_GT(val1, val2) ARROW_UNUSED(val1 > val2)
Review comment:
I'm not saying the comparison _would_ have a side-effect, just that the compiler wouldn't know about it and therefore couldn't entirely eliminate the comparison code.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529850927
##########
File path: cpp/build-support/cpplint.py
##########
@@ -4560,7 +4560,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
CheckCommaSpacing(filename, clean_lines, linenum, error)
CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error)
CheckSpacingForFunctionCall(filename, clean_lines, linenum, error)
- CheckCheck(filename, clean_lines, linenum, error)
+ #CheckCheck(filename, clean_lines, linenum, error)
Review comment:
Yes, this is a lint check which fails for `DCHECK(x == y)` (consider using `DCHECK_EQ(x, y)`)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530187510
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
Review comment:
Would there be a less cryptic way to implement the whole thing?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
Review comment:
Same question then :-)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8735: ARROW-6781: [C++] Improve and consoldiate CHECK macros
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#issuecomment-731634539
https://issues.apache.org/jira/browse/ARROW-6781
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530348934
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -86,56 +67,40 @@ enum class ArrowLogLevel : int {
#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
-#ifdef NDEBUG
-#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
+#define ARROW_LOG_FAILED_STATUS(status, ...) \
+ ARROW_LOG(FATAL) << " Operation failed: " << ARROW_STRINGIFY(__VA_ARGS__) << "\n" \
+ << " Bad status: " << _s.ToString()
+
+// If the status is bad, CHECK immediately, appending the status to the logged
+// message.
+#define ARROW_CHECK_OK(...) \
+ for (::arrow::Status _s = ::arrow::internal::GenericToStatus((__VA_ARGS__)); \
Review comment:
The last tokens of a `do... while` loop are the loop condition, which precludes providing a stream for extra error messages.
`for` supports a scoped decl, which `while` doesn't. If we wanted to avoid `for`, we could introduce a variable as in `ARROW_ASSIGN_OR_RAISE`. After some macro expansion that'd look like:
```c++
Status _s_56 = Thing();
if (ARROW_PREDICT_FALSE(!_s_56.ok()))
ARROW_LOG(FATAL) << _s.message() << "Extra messages";
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529862621
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
Review comment:
The assert condition is tested [without decoration](https://github.com/apache/arrow/pull/8735/files#diff-e4af9b24769d2da2516cca1deb194f67f3fdbf692e6486c0e7149279af01cfb6R83). This avoids some compiler warnings and suspicion that destructuring might slow down the fast path (the check passes).
Destructuring operates on another copy of the condition expression for printing purposes only.
It relies on the slightly higher precedence of `<=` compared to the other comparison operators. After macro expansion, the [intercepted comparison](https://github.com/apache/arrow/pull/8735/files#diff-e4af9b24769d2da2516cca1deb194f67f3fdbf692e6486c0e7149279af01cfb6R85) is as follows:
```
InterceptComparison() <= x > y
((InterceptComparison() <= x) > y) // parentheses for clarity
```
The inner parenthesis captures `x`, then the outer parenthesis captures `y`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529855297
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
Review comment:
These bound expressions must be contextually convertible to bool to enable `DCHECK(x >= 0 && x < 8)`. (It can't be destructured for better printing, but it should still be a valid assertion.) If the conversion to bool is not provided, then `bound_expr && x < 8` breaks compilation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530408123
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
Review comment:
Ok, but destructuring in the first place shouldn't be required. I guess you want `DCHECK(a == b)` to function exactly like `DCHECK_EQ(a, b)`, but that doesn't sound required to me.
Otherwise, please at least explain all this in comments, because it's extremely cryptic at first sight.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz closed pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz closed pull request #8735:
URL: https://github.com/apache/arrow/pull/8735
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r530369496
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
Review comment:
This is the approach used by the [Catch2](https://github.com/catchorg/Catch2) test framework's assertion macro. I'm not aware of another way to destructure a comparison's operands
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529852117
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -22,22 +22,21 @@
// The LLVM IR code doesn't have an NDEBUG mode. And, it shouldn't include references to
// streams or stdc++. So, making the DCHECK calls void in that case.
-#define ARROW_IGNORE_EXPR(expr) ((void)(expr))
-
-#define DCHECK(condition) ARROW_IGNORE_EXPR(condition)
-#define DCHECK_OK(status) ARROW_IGNORE_EXPR(status)
-#define DCHECK_EQ(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_NE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LT(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GT(val1, val2) ARROW_IGNORE_EXPR(val1)
+#define DCHECK(condition) ARROW_UNUSED(condition)
+#define DCHECK_OK(status) ARROW_UNUSED(status)
+#define DCHECK_EQ(val1, val2) ARROW_UNUSED(val1 == val2)
+#define DCHECK_NE(val1, val2) ARROW_UNUSED(val1 != val2)
+#define DCHECK_LE(val1, val2) ARROW_UNUSED(val1 <= val2)
+#define DCHECK_LT(val1, val2) ARROW_UNUSED(val1 < val2)
+#define DCHECK_GE(val1, val2) ARROW_UNUSED(val1 >= val2)
+#define DCHECK_GT(val1, val2) ARROW_UNUSED(val1 > val2)
Review comment:
okay, but since these are debug checks anyway the giving them side effects results in differing behavior between release and debug. In general it's buggy to have non trivial code inside a debug check
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] bkietz commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529853863
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -86,56 +67,40 @@ enum class ArrowLogLevel : int {
#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
-#ifdef NDEBUG
-#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
+#define ARROW_LOG_FAILED_STATUS(status, ...) \
+ ARROW_LOG(FATAL) << " Operation failed: " << ARROW_STRINGIFY(__VA_ARGS__) << "\n" \
+ << " Bad status: " << _s.ToString()
+
+// If the status is bad, CHECK immediately, appending the status to the logged
+// message.
+#define ARROW_CHECK_OK(...) \
+ for (::arrow::Status _s = ::arrow::internal::GenericToStatus((__VA_ARGS__)); \
Review comment:
Yes, it allows us to confine `_s` to the scope of the loop body. The loop body is a single statement with no braces, allowing `*CHECK_OK()` to accept a stream.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8735: ARROW-6781: [C++] Improve and consolidate CHECK macros
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8735:
URL: https://github.com/apache/arrow/pull/8735#discussion_r529636621
##########
File path: cpp/build-support/cpplint.py
##########
@@ -4560,7 +4560,7 @@ def CheckStyle(filename, clean_lines, linenum, file_extension, nesting_state,
CheckCommaSpacing(filename, clean_lines, linenum, error)
CheckBracesSpacing(filename, clean_lines, linenum, nesting_state, error)
CheckSpacingForFunctionCall(filename, clean_lines, linenum, error)
- CheckCheck(filename, clean_lines, linenum, error)
+ #CheckCheck(filename, clean_lines, linenum, error)
Review comment:
Is there a reason this is commented out?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -86,56 +67,40 @@ enum class ArrowLogLevel : int {
#define ARROW_CHECK_GE(val1, val2) ARROW_CHECK((val1) >= (val2))
#define ARROW_CHECK_GT(val1, val2) ARROW_CHECK((val1) > (val2))
-#ifdef NDEBUG
-#define ARROW_DFATAL ::arrow::util::ArrowLogLevel::ARROW_WARNING
+#define ARROW_LOG_FAILED_STATUS(status, ...) \
+ ARROW_LOG(FATAL) << " Operation failed: " << ARROW_STRINGIFY(__VA_ARGS__) << "\n" \
+ << " Bad status: " << _s.ToString()
+
+// If the status is bad, CHECK immediately, appending the status to the logged
+// message.
+#define ARROW_CHECK_OK(...) \
+ for (::arrow::Status _s = ::arrow::internal::GenericToStatus((__VA_ARGS__)); \
Review comment:
Is there a particular reason for using a for loop?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -22,22 +22,21 @@
// The LLVM IR code doesn't have an NDEBUG mode. And, it shouldn't include references to
// streams or stdc++. So, making the DCHECK calls void in that case.
-#define ARROW_IGNORE_EXPR(expr) ((void)(expr))
-
-#define DCHECK(condition) ARROW_IGNORE_EXPR(condition)
-#define DCHECK_OK(status) ARROW_IGNORE_EXPR(status)
-#define DCHECK_EQ(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_NE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_LT(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GE(val1, val2) ARROW_IGNORE_EXPR(val1)
-#define DCHECK_GT(val1, val2) ARROW_IGNORE_EXPR(val1)
+#define DCHECK(condition) ARROW_UNUSED(condition)
+#define DCHECK_OK(status) ARROW_UNUSED(status)
+#define DCHECK_EQ(val1, val2) ARROW_UNUSED(val1 == val2)
+#define DCHECK_NE(val1, val2) ARROW_UNUSED(val1 != val2)
+#define DCHECK_LE(val1, val2) ARROW_UNUSED(val1 <= val2)
+#define DCHECK_LT(val1, val2) ARROW_UNUSED(val1 < val2)
+#define DCHECK_GE(val1, val2) ARROW_UNUSED(val1 >= val2)
+#define DCHECK_GT(val1, val2) ARROW_UNUSED(val1 > val2)
Review comment:
Hmm... if the comparisons are not trivial (for example they call a function that the compiler can't see whether it has side effects), then not all code will be eliminated?
Perahsp instead something like `ARROW_UNUSED(val1), ARROW_UNUSED(val2)`?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
Review comment:
Hmm... why?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ explicit constexpr operator bool() const { return false; }
+
+ const Lhs& lhs;
+ };
+
+ template <typename Lhs>
+ BoundLhs<Lhs> operator<=(const Lhs& lhs) && {
Review comment:
Same question here... Don't you want to used regular method names?
##########
File path: cpp/src/arrow/util/logging.h
##########
@@ -202,48 +169,81 @@ class ARROW_EXPORT ArrowLog : public ArrowLogBase {
private:
ARROW_DISALLOW_COPY_AND_ASSIGN(ArrowLog);
+ std::ostream* Stream() override;
+
// Hide the implementation of log provider by void *.
// Otherwise, lib user may define the same macro to use the correct header file.
void* logging_provider_;
/// True if log messages should be logged and false if they should be ignored.
bool is_enabled_;
static ArrowLogLevel severity_threshold_;
-
- protected:
- std::ostream& Stream() override;
};
-// This class make ARROW_CHECK compilation pass to change the << operator to void.
-// This class is copied from glog.
-class ARROW_EXPORT Voidify {
- public:
- Voidify() {}
- // This has to be an operator with a precedence lower than << but
- // higher than ?:
- void operator&(ArrowLogBase&) {}
-};
+struct ARROW_EXPORT InterceptComparison {
+ template <typename NotBinaryOrNotPrintable>
+ static ArrowLogBase&& PrintOperands(const NotBinaryOrNotPrintable&,
+ ArrowLogBase&& log) {
+ return std::move(log);
+ }
-namespace detail {
+ template <typename Lhs, typename Rhs>
+ struct BoundLhsRhs {
+ explicit constexpr operator bool() const { return false; }
+ const Lhs& lhs;
+ const Rhs& rhs;
+ };
+
+ template <typename Lhs, typename Rhs>
+ static auto PrintOperands(const BoundLhsRhs<Lhs, Rhs>& bound, ArrowLogBase&& log)
+ -> decltype(std::move(log) << bound.lhs << bound.rhs) {
+ return std::move(log) << "\n left: " << bound.lhs << "\n right: " << bound.rhs
+ << "\n";
+ }
-/// @brief A helper for the nil log sink.
-///
-/// Using this helper is analogous to sending log messages to /dev/null:
-/// nothing gets logged.
-class NullLog {
- public:
- /// The no-op output operator.
- ///
- /// @param [in] t
- /// The object to send into the nil sink.
- /// @return Reference to the updated object.
- template <class T>
- NullLog& operator<<(const T& t) {
- return *this;
+ template <typename Lhs>
+ struct BoundLhs {
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator==(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator!=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator>=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
+
+ template <typename Rhs>
+ BoundLhsRhs<Lhs, Rhs> operator<=(const Rhs& rhs) && {
+ return {lhs, rhs};
+ }
Review comment:
Why all these operators? They don't look like they perform the advertised operations.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org