This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new e8c2e70605f GH-51456: [C++] Make ABI independent of
ARROW_EXTRA_ERROR_CONTEXT (#51458)
e8c2e70605f is described below
commit e8c2e70605ffa89edfdbc701ef68124e3cf995ba
Author: Antoine Pitrou <[email protected]>
AuthorDate: Tue Sep 22 16:01:06 2026 +0200
GH-51456: [C++] Make ABI independent of ARROW_EXTRA_ERROR_CONTEXT (#51458)
### What changes are included in this PR?
Allow applications compiled with `ARROW_EXTRA_ERROR_CONTEXT` enabled to
link against libarrow compiled with `ARROW_EXTRA_ERROR_CONTEXT` disabled, and
vice-versa.
Also improve unit tests slightly.
### Are these changes tested?
The original issue was tested manually.
### Are there any user-facing changes?
No.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [x] Human
- [ ] AI
**Reviewed before submission by:**
- [x] Human
- [ ] AI
- [ ] Not reviewed
* GitHub Issue: #51456
Authored-by: Antoine Pitrou <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/status.cc | 12 +++---------
cpp/src/arrow/status.h | 2 --
cpp/src/arrow/status_test.cc | 39 +++++++++++++++++++++++----------------
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/cpp/src/arrow/status.cc b/cpp/src/arrow/status.cc
index 4730bca8c6c..50eb001c3a7 100644
--- a/cpp/src/arrow/status.cc
+++ b/cpp/src/arrow/status.cc
@@ -16,9 +16,7 @@
#include <cctype>
#include <cstdlib>
#include <iostream>
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
-# include <sstream>
-#endif
+#include <sstream>
#include "arrow/util/logging.h"
@@ -126,14 +124,13 @@ std::string Status::ToString() const {
std::string Status::ToStringWithoutContextLines() const {
auto message = ToString();
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
while (true) {
auto last_new_line_position = message.rfind("\n");
if (last_new_line_position == std::string::npos) {
break;
}
// Check for the pattern ":\d+ " (colon followed by one or more digits and
a space)
- // to identify context lines in the format "filename:line expr"
+ // to identify context lines in the format "filename:line expr"
auto colon_position = message.find(":", last_new_line_position);
if (colon_position == std::string::npos) {
break;
@@ -155,7 +152,6 @@ std::string Status::ToStringWithoutContextLines() const {
}
message = message.substr(0, last_new_line_position);
}
-#endif
return message;
}
@@ -186,17 +182,15 @@ void Status::Warn(const std::string& message) const {
ARROW_LOG(WARNING) << message << ": " << ToString();
}
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
void Status::AddContextLine(const char* filename, int line, const char* expr) {
ARROW_CHECK(!ok()) << "Cannot add context line to ok status";
std::stringstream ss;
- ss << "\n" << filename << ":" << line << " " << expr;
+ ss << "\n" << filename << ":" << line << " " << expr;
if (state_->is_constant) {
// We can't add context lines to a StatusConstant's state, so copy it now
state_ = new State{code(), /*is_constant=*/false, message(), detail()};
}
state_->msg += ss.str();
}
-#endif
} // namespace arrow
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 8907d32ff7d..5757440cee7 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -393,9 +393,7 @@ class ARROW_EXPORT [[nodiscard]] Status : public
util::EqualityComparable<Status
[[noreturn]] void Abort() const;
[[noreturn]] void Abort(const std::string& message) const;
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
void AddContextLine(const char* filename, int line, const char* expr);
-#endif
private:
struct State {
diff --git a/cpp/src/arrow/status_test.cc b/cpp/src/arrow/status_test.cc
index 72998cba78f..a38e815aa8c 100644
--- a/cpp/src/arrow/status_test.cc
+++ b/cpp/src/arrow/status_test.cc
@@ -28,6 +28,13 @@
namespace arrow {
+// Keep at top of the file to make line number in asserted error message
stable.
+template <typename StatusLike>
+Status ReturnNotOk(StatusLike&& status_like) {
+ RETURN_NOT_OK(status_like);
+ return Status::OK();
+}
+
namespace {
class TestStatusDetail : public StatusDetail {
@@ -314,40 +321,41 @@ std::string StripContext(const std::string& message) {
}
TEST(StatusTest, ReturnIfNotOk) {
- auto f = [](auto v) {
- RETURN_NOT_OK(v);
- return Status::OK();
- };
-
auto ok_status = Status::OK();
auto error_status = Status::IOError("some message");
Status st;
- st = f(ok_status);
+ st = ReturnNotOk(ok_status);
ASSERT_TRUE(st.ok());
- st = f(error_status);
+ st = ReturnNotOk(error_status);
ASSERT_EQ(st.code(), StatusCode::IOError);
ASSERT_EQ(StripContext(st.message()), error_status.message());
+#ifdef ARROW_EXTRA_ERROR_CONTEXT
+ ASSERT_THAT(st.message(), ::testing::EndsWith("status_test.cc:34
status_like"));
+#endif
- st = f(Result<int>(42));
+ st = ReturnNotOk(Result<int>(42));
ASSERT_TRUE(st.ok());
- st = f(Result<int>(error_status));
+ st = ReturnNotOk(Result<int>(error_status));
ASSERT_EQ(st.code(), StatusCode::IOError);
ASSERT_EQ(StripContext(st.message()), error_status.message());
- st = f(my_namespace::StatusLike{42});
+ st = ReturnNotOk(my_namespace::StatusLike{42});
ASSERT_TRUE(st.ok());
- st = f(my_namespace::StatusLike{43});
+ st = ReturnNotOk(my_namespace::StatusLike{43});
ASSERT_EQ(st.code(), StatusCode::UnknownError);
ASSERT_EQ(StripContext(st.message()), "StatusLike: 43");
}
-#ifdef ARROW_EXTRA_ERROR_CONTEXT
-TEST(StatusTest, ToStringWithoutContextLines) {
+TEST(StatusTest, ContextLines) {
Status status = Status::IOError("base error");
- status.AddContextLine("file1.cc", 42, "expr");
- status.AddContextLine("file2.cc", 100, "expr");
+ status.AddContextLine("file1.cc", 42, "expr1");
+ status.AddContextLine("file2.cc", 100, "expr2");
+ ASSERT_EQ(status.ToString(),
+ R"(IOError: base error
+file1.cc:42 expr1
+file2.cc:100 expr2)");
ASSERT_EQ(status.ToStringWithoutContextLines(), "IOError: base error");
Status status2(StatusCode::Invalid,
@@ -357,6 +365,5 @@ TEST(StatusTest, ToStringWithoutContextLines) {
ASSERT_EQ(status2.ToStringWithoutContextLines(),
"Invalid: Error message\nThis line has: a colon but no digits");
}
-#endif
} // namespace arrow
