This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git
The following commit(s) were added to refs/heads/main by this push:
new 8e35d056 fix: qualify Iceberg names in public error macros (#1026)
8e35d056 is described below
commit 8e35d0560419be6b422dfa95c74289a172e21373
Author: Manu Zhang <[email protected]>
AuthorDate: Wed Oct 21 15:31:32 2026 +0800
fix: qualify Iceberg names in public error macros (#1026)
ICEBERG_RETURN_UNEXPECTED, and through it ICEBERG_ASSIGN_OR_RAISE, named
Error without its namespace. ICEBERG_PRECHECK and ICEBERG_CHECK did the
same with InvalidArgument and ValidationFailed, and the ICEBERG_BUILDER_*
macros with ErrorKind. Those names resolve only inside namespace iceberg,
so code that includes the installed headers could not use the macros.
Qualify them with ::iceberg::, as the logging macros already do, and use
::iceberg:: in the exception macros as well.
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
src/iceberg/exception.h | 14 ++--
src/iceberg/test/CMakeLists.txt | 1 +
src/iceberg/test/error_macros_test.cc | 133 ++++++++++++++++++++++++++++++++++
src/iceberg/util/error_collector.h | 20 ++---
src/iceberg/util/macros.h | 39 +++++-----
5 files changed, 173 insertions(+), 34 deletions(-)
diff --git a/src/iceberg/exception.h b/src/iceberg/exception.h
index 0333eb12..a182cb0d 100644
--- a/src/iceberg/exception.h
+++ b/src/iceberg/exception.h
@@ -30,6 +30,13 @@
#include "iceberg/iceberg_export.h"
+#define ICEBERG_CHECK_OR_DIE(condition, ...) \
+ do { \
+ if (!(condition)) [[unlikely]] { \
+ throw ::iceberg::IcebergError(std::format(__VA_ARGS__)); \
+ } \
+ } while (0)
+
namespace iceberg {
/// \brief Base exception class for exceptions thrown by the Iceberg library.
@@ -44,11 +51,4 @@ class ICEBERG_EXPORT ExpressionError : public IcebergError {
explicit ExpressionError(const std::string& what) : IcebergError(what) {}
};
-#define ICEBERG_CHECK_OR_DIE(condition, ...) \
- do { \
- if (!(condition)) [[unlikely]] { \
- throw iceberg::IcebergError(std::format(__VA_ARGS__)); \
- } \
- } while (0)
-
} // namespace iceberg
diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt
index 891ce374..8b9dd2e9 100644
--- a/src/iceberg/test/CMakeLists.txt
+++ b/src/iceberg/test/CMakeLists.txt
@@ -135,6 +135,7 @@ add_iceberg_test(util_test
data_file_set_test.cc
decimal_test.cc
endian_test.cc
+ error_macros_test.cc
expected_test.cc
file_io_test.cc
formatter_test.cc
diff --git a/src/iceberg/test/error_macros_test.cc
b/src/iceberg/test/error_macros_test.cc
new file mode 100644
index 00000000..f0b1f53e
--- /dev/null
+++ b/src/iceberg/test/error_macros_test.cc
@@ -0,0 +1,133 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "iceberg/exception.h"
+#include "iceberg/result.h"
+#include "iceberg/test/matchers.h"
+#include "iceberg/util/error_collector.h"
+#include "iceberg/util/macros.h"
+
+namespace {
+
+using ::iceberg::HasErrorMessage;
+using ::iceberg::HasValue;
+using ::iceberg::IsError;
+using ::iceberg::IsOk;
+
+// The macros expand here, in a namespace with its own nested iceberg
namespace, so a
+// macro that names iceberg::X instead of ::iceberg::X fails to compile.
+namespace caller {
+
+namespace iceberg {}
+
+::iceberg::Result<int> Parse(bool valid) {
+ if (!valid) {
+ return ::iceberg::InvalidArgument("Invalid input");
+ }
+ return 42;
+}
+
+::iceberg::Result<int> AddOne(bool valid) {
+ ICEBERG_ASSIGN_OR_RAISE(auto value, Parse(valid));
+ return value + 1;
+}
+
+::iceberg::Status Validate(bool valid) {
+ ICEBERG_RETURN_UNEXPECTED(Parse(valid));
+ return {};
+}
+
+::iceberg::Status CheckRange(int value) {
+ ICEBERG_PRECHECK(value > 0, "Value must be positive, got {}", value);
+ ICEBERG_CHECK(value < 100, "Value must be below 100, got {}", value);
+ return {};
+}
+
+int AddOneOrThrow(bool valid) {
+ ICEBERG_ASSIGN_OR_THROW(auto value, Parse(valid));
+ return value + 1;
+}
+
+int CheckPositiveOrDie(int value) {
+ ICEBERG_CHECK_OR_DIE(value > 0, "Value must be positive, got {}", value);
+ return value;
+}
+
+class TestBuilder : public ::iceberg::ErrorCollector {
+ public:
+ TestBuilder& Set(int value) {
+ ICEBERG_BUILDER_CHECK(value > 0, "Value must be positive, got {}", value);
+ ICEBERG_BUILDER_ASSIGN_OR_RETURN(value_, Parse(value < 100));
+ ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR(value_, Parse(value < 50),
+ "Value {} is too large",
value);
+ return *this;
+ }
+
+ private:
+ int value_ = 0;
+};
+
+} // namespace caller
+
+using caller::AddOne;
+using caller::AddOneOrThrow;
+using caller::CheckPositiveOrDie;
+using caller::CheckRange;
+using caller::TestBuilder;
+using caller::Validate;
+
+} // namespace
+
+TEST(ErrorMacrosTest, AssignOrRaise) {
+ EXPECT_THAT(AddOne(true), HasValue(::testing::Eq(43)));
+ EXPECT_THAT(AddOne(false), IsError(iceberg::ErrorKind::kInvalidArgument));
+}
+
+TEST(ErrorMacrosTest, ReturnUnexpected) {
+ EXPECT_THAT(Validate(true), IsOk());
+ EXPECT_THAT(Validate(false), HasErrorMessage("Invalid input"));
+}
+
+TEST(ErrorMacrosTest, PrecheckAndCheck) {
+ EXPECT_THAT(CheckRange(1), IsOk());
+ EXPECT_THAT(CheckRange(0), IsError(iceberg::ErrorKind::kInvalidArgument));
+ EXPECT_THAT(CheckRange(100), IsError(iceberg::ErrorKind::kValidationFailed));
+}
+
+TEST(ErrorMacrosTest, AssignOrThrow) {
+ EXPECT_EQ(AddOneOrThrow(true), 43);
+ EXPECT_THROW(AddOneOrThrow(false), iceberg::IcebergError);
+}
+
+TEST(ErrorMacrosTest, CheckOrDie) {
+ EXPECT_EQ(CheckPositiveOrDie(1), 1);
+ EXPECT_THROW(CheckPositiveOrDie(0), iceberg::IcebergError);
+}
+
+TEST(ErrorMacrosTest, BuilderMacros) {
+ EXPECT_THAT(TestBuilder().Set(1).CheckErrors(), IsOk());
+ EXPECT_THAT(TestBuilder().Set(0).CheckErrors(),
+ HasErrorMessage("Value must be positive, got 0"));
+ EXPECT_THAT(TestBuilder().Set(150).CheckErrors(), HasErrorMessage("Invalid
input"));
+ EXPECT_THAT(TestBuilder().Set(70).CheckErrors(),
+ HasErrorMessage("Value 70 is too large"));
+}
diff --git a/src/iceberg/util/error_collector.h
b/src/iceberg/util/error_collector.h
index 9eb8de15..d8834f28 100644
--- a/src/iceberg/util/error_collector.h
+++ b/src/iceberg/util/error_collector.h
@@ -29,8 +29,8 @@
#include "iceberg/iceberg_export.h"
#include "iceberg/result.h"
-namespace iceberg {
-
+// The builder macros name Iceberg symbols with their full namespace, so
builders outside
+// namespace iceberg can use them.
#define ICEBERG_BUILDER_RETURN_IF_ERROR(result) \
if (auto&& result_name = result; !result_name) [[unlikely]] { \
errors_.emplace_back(std::move(result_name.error())); \
@@ -49,7 +49,7 @@ namespace iceberg {
#define ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR_IMPL(result_name, lhs,
rexpr, ...) \
auto&& result_name = (rexpr);
\
if (!result_name) [[unlikely]] {
\
- AddError(ErrorKind::kInvalidArgument, __VA_ARGS__);
\
+ AddError(::iceberg::ErrorKind::kInvalidArgument, __VA_ARGS__);
\
return *this;
\
}
\
lhs = std::move(result_name.value());
@@ -58,14 +58,16 @@ namespace iceberg {
ICEBERG_BUILDER_ASSIGN_OR_RETURN_WITH_ERROR_IMPL( \
ICEBERG_ASSIGN_OR_RAISE_NAME(result_, __COUNTER__), lhs, rexpr,
__VA_ARGS__)
-#define ICEBERG_BUILDER_CHECK(expr, ...) \
- do { \
- if (!(expr)) [[unlikely]] { \
- AddError(ErrorKind::kInvalidArgument, __VA_ARGS__); \
- return *this; \
- } \
+#define ICEBERG_BUILDER_CHECK(expr, ...) \
+ do { \
+ if (!(expr)) [[unlikely]] { \
+ AddError(::iceberg::ErrorKind::kInvalidArgument, __VA_ARGS__); \
+ return *this; \
+ } \
} while (false)
+namespace iceberg {
+
/// \brief Base class for collecting errors in the builder pattern.
///
/// This class equips builders with error accumulation capabilities to make it
easy
diff --git a/src/iceberg/util/macros.h b/src/iceberg/util/macros.h
index ca481a93..cd046ba6 100644
--- a/src/iceberg/util/macros.h
+++ b/src/iceberg/util/macros.h
@@ -21,15 +21,18 @@
/// \file iceberg/util/macros.h
/// \brief Define common Iceberg utility macros.
+///
+/// The macros name Iceberg symbols with their full namespace, so code outside
namespace
+/// iceberg can use them.
#include <cassert>
#include "iceberg/exception.h"
#include "iceberg/result.h"
-#define ICEBERG_RETURN_UNEXPECTED(expr) \
- if (auto&& result_name = expr; !result_name) [[unlikely]] { \
- return iceberg::unexpected<Error>(result_name.error()); \
+#define ICEBERG_RETURN_UNEXPECTED(expr) \
+ if (auto&& result_name = expr; !result_name) [[unlikely]] { \
+ return ::iceberg::unexpected<::iceberg::Error>(result_name.error()); \
}
#define ICEBERG_ASSIGN_OR_RAISE_IMPL(result_name, lhs, rexpr) \
@@ -49,26 +52,26 @@
#define ICEBERG_DCHECK(expr, message) assert((expr) && (message))
// Macro for precondition checks, usually used for function arguments
-#define ICEBERG_PRECHECK(expr, ...) \
- do { \
- if (!(expr)) [[unlikely]] { \
- return InvalidArgument(__VA_ARGS__); \
- } \
+#define ICEBERG_PRECHECK(expr, ...) \
+ do { \
+ if (!(expr)) [[unlikely]] { \
+ return ::iceberg::InvalidArgument(__VA_ARGS__); \
+ } \
} while (0)
// Macro for state checks, usually used for unexpected states
-#define ICEBERG_CHECK(expr, ...) \
- do { \
- if (!(expr)) [[unlikely]] { \
- return ValidationFailed(__VA_ARGS__); \
- } \
+#define ICEBERG_CHECK(expr, ...) \
+ do { \
+ if (!(expr)) [[unlikely]] { \
+ return ::iceberg::ValidationFailed(__VA_ARGS__); \
+ } \
} while (0)
-#define ERROR_TO_EXCEPTION(error) \
- if (error.kind == iceberg::ErrorKind::kInvalidExpression) { \
- throw iceberg::ExpressionError(error.message); \
- } else { \
- throw iceberg::IcebergError(error.message); \
+#define ERROR_TO_EXCEPTION(error) \
+ if (error.kind == ::iceberg::ErrorKind::kInvalidExpression) { \
+ throw ::iceberg::ExpressionError(error.message); \
+ } else { \
+ throw ::iceberg::IcebergError(error.message); \
}
#define ICEBERG_THROW_NOT_OK(expr) \
