Skip to content

Commit 2cb7357

Browse files
move location_'s assignment to the constructor with test cases
1 parent ecc17a9 commit 2cb7357

2 files changed

Lines changed: 21 additions & 6 deletions

File tree

‎src/iceberg/logging/logger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ struct ICEBERG_EXPORT LogMessage {
8585
/// Location() (e.g. to forward a caller's std::source_location).
8686
class ICEBERG_EXPORT LogMessage::Builder {
8787
public:
88-
explicit Builder(LogLevel level) : level_(level) {}
88+
explicit Builder(LogLevel level) : level_(level), location_(std::source_location::current()) {}
8989

9090
/// \brief Set the already-formatted message text.
9191
Builder& Message(std::string message) {
@@ -117,7 +117,7 @@ class ICEBERG_EXPORT LogMessage::Builder {
117117
LogLevel level_;
118118
std::string message_;
119119
// `location_` is a trivially copyable members no need to move.
120-
std::source_location location_ = std::source_location::current();
120+
std::source_location location_;
121121
std::vector<LogAttribute> attributes_;
122122
};
123123

‎src/iceberg/test/logger_test.cc‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
#include <atomic>
2323
#include <memory>
24+
#include <string_view>
2425
#include <thread>
2526
#include <tuple>
2627
#include <vector>
@@ -429,10 +430,24 @@ TEST(LoggerTest, BuilderDefaultsAndEmitToSink) {
429430
EXPECT_NE(records[0].location.line(), 0u); // location defaulted at build site
430431
}
431432

432-
TEST(LoggerTest, BuilderLocationOverride) {
433-
auto loc = std::source_location::current();
434-
auto record = LogMessage::Builder(LogLevel::kDebug).Location(loc).Build();
435-
EXPECT_EQ(record.location.line(), loc.line());
433+
// location_ is initialized in the Builder constructor, so when Location() is not
434+
// called the default points at the constructor itself (in logger.h), not at the
435+
// caller and not at line 0.
436+
TEST(LoggerTest, BuilderDefaultLocationIsConstructorSite) {
437+
auto record = LogMessage::Builder(LogLevel::kInfo).Message("m").Build();
438+
EXPECT_NE(record.location.line(), 0u);
439+
EXPECT_NE(std::string_view(record.location.file_name()).find("logger.h"),
440+
std::string_view::npos);
441+
EXPECT_NE(std::string_view(record.location.function_name()).find("Builder"),
442+
std::string_view::npos);
443+
}
444+
445+
// Location() replaces the constructor default with the caller's site (file + line).
446+
TEST(LoggerTest, BuilderLocationOverrideUsesCallerSite) {
447+
auto caller = std::source_location::current();
448+
auto record = LogMessage::Builder(LogLevel::kDebug).Location(caller).Build();
449+
EXPECT_EQ(record.location.line(), caller.line());
450+
EXPECT_STREQ(record.location.file_name(), caller.file_name());
436451
}
437452

438453
} // namespace iceberg

0 commit comments

Comments
 (0)