feat!: make Logger a value type with a public ILogger sink - #625
Merged
Conversation
andiwand
force-pushed
the
feat/logger-value-type
branch
from
July 27, 2026 19:26
46e03ee to
5ce4a5d
Compare
Logger a value typeLogger a value type with a public ILogger sink
`Logger` was the one public interface type that was not a value handle: it was an abstract base handed out as `Logger &` or `std::shared_ptr<Logger>`, and the two spellings had drifted apart across the API — `file.hpp` and `odr.hpp` took `Logger &`, `html.hpp` and `http_server.hpp` took `std::shared_ptr<Logger>`. The sink becomes `odr::ILogger`, a public interface, and `Logger` becomes a copyable handle over a `shared_ptr` to it, mirroring `File`, `DecodedFile` and `Document`. Both spellings collapse into a single `const Logger &` parameter, defaulted to `Logger::null()`. `ILogger` stays in the public header rather than moving to `internal::abstract`: routing diagnostics is something a user of the library legitimately wants to do, and an extension point behind an internal header is not one. Logging is now extensible from the bindings too. Python subclasses `pyodr.ILogger`; Java implements `app.opendocument.core.ILogger`. Both gained a `logger` argument on the entry points that take one, so a custom sink can actually be handed to the library. `ILogger::log` is non-const, matching `std::ostream`, whose writing operations are all non-const while its state queries are const. The old `const` was vacuous anyway — `*m_output` on a const `unique_ptr` yields a non-const `ostream &` — and it forced every implementation to declare its state `mutable`. BREAKING CHANGE: - `Logger` is `final`; custom sinks implement `odr::ILogger` and are wrapped with `Logger(std::shared_ptr<ILogger>)`. - `Logger::null()` returns a `Logger` by value instead of `Logger &`. - `Logger::create_null()` is gone — use `Logger::null()` (all instances share one sink, so it no longer allocates). - `create_stdio` returns `Logger`, not `std::unique_ptr<Logger>`. - `create_tee` returns `Logger` and takes `const std::vector<Logger> &` instead of `std::vector<std::shared_ptr<Logger>>`. - every API taking `Logger &` or `std::shared_ptr<Logger>` now takes `const Logger &`. Parsers that held a borrowed `const Logger *` now hold a `Logger` by value, so a logger outliving its parser is no longer the caller's problem. `TeeLogger` no longer flushes its children from its destructor: each child handle drops its sink on destruction and a buffering sink flushes itself, so the tee only stood to call into an already-torn-down sink. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KbnNCsz6rkVKHW2MWL47m5
andiwand
force-pushed
the
feat/logger-value-type
branch
from
July 27, 2026 19:34
5ce4a5d to
87f7cf3
Compare
andiwand
enabled auto-merge (squash)
July 27, 2026 19:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Clears the first of the two
Breaking API changes (next major)items indocs/design/README.md, ahead of the v6 tag.Why
Loggerwas the one public interface type that was not a value handle. It was anabstract base handed out as
Logger &orstd::shared_ptr<Logger>, and the twospellings had drifted apart across the API:
file.hpp,odr.hppLogger &logger = Logger::null()html.hpp,http_server.hppstd::shared_ptr<Logger> logger = Logger::create_null()What
The sink becomes
odr::ILoggerandLoggerbecomes a copyable handle over ashared_ptrto it, mirroringFile,DecodedFileandDocument(including theimpl()accessor). Both spellings collapse into a singleconst Logger &,defaulted to
Logger::null().ILoggerlives in the public header rather thaninternal::abstract. Routingdiagnostics somewhere is a thing a library user legitimately wants to do, and an
extension point that requires including an internal header is not one.
Parsers that held a borrowed
const Logger *m_logger{nullptr}now hold aLoggerby value, so a logger outliving its parser stops being the caller's problem.
Bindings can implement a sink too
Both bindings previously dropped
Loggerentirely. Now:Entry points that take a logger in C++ gained an optional
loggerargument.Two teardown traps the Python trampoline had to work around — both invisible to a
happy-path test, and both hit while writing this:
shared_ptrowning a reference to the Pythonobject. Without it,
Logger(MySink())leaves C++ holding a dead object and thenext call dies with
Tried to call pure virtual function.flush()dispatches viapy::get_override, notPYBIND11_OVERRIDE_PURE. Sinksare flushed from destructors, which can run after the Python object is gone, and
throwing from a destructor aborts the process.
On the Java side
JavaLoggerattaches/detaches the calling thread (log callsarrive on whatever thread the library is working on) and clears any exception the
sink throws rather than leaving it pending — a logger must not derail the
operation it is reporting on.
ILogger::logis non-conststd::ostreamis the precedent and it argues the same way:operator<<,write,putandflushare non-const, whilegood/fail/rdstateare const. Writingmutates the sink.
The old
constwas also vacuous —*m_outputon aconst std::unique_ptryieldsa non-const
std::ostream &, soStdioLoggermutated freely from a const method.All it achieved was forcing implementations to declare their state
mutable, asthe test sink in
logger_test.cpphad to before this.will_logstays const; itis a genuine query.
The handle's
log()/flush()remain const — const handle, mutable sink, exactlylike
shared_ptr. That is what lets every API takeconst Logger &.Breaking changes
Loggerisfinal; custom sinks implementodr::ILoggerand are wrapped withLogger(std::shared_ptr<ILogger>).Logger::null()returns aLoggerby value instead ofLogger &.Logger::create_null()is gone — useLogger::null(). All instances share onesink, so it no longer allocates.
create_stdioreturnsLogger, notstd::unique_ptr<Logger>.create_teereturnsLoggerand takesconst std::vector<Logger> &.Logger &orstd::shared_ptr<Logger>now takesconst Logger &.The
ODR_*macros are unchanged; call sites just drop the*(
ODR_INFO(*logger, …)→ODR_INFO(logger, …)).TeeLoggerno longer flushes its children from its destructor. It was redundant —each child handle drops its sink on destruction and a buffering sink flushes
itself — and it was the path that called into an already-torn-down Python sink.
Test plan
logger_test.cppwent from one smoke test to nine, covering what the valuesemantics newly promise: copies share the sink, the default handle is the null
logger, level filtering, tee fan-out, empty-tee throw, a null sink rejected, and
an out-of-library
ILoggerdriven through the handle and through a tee.test_logger.pyimplementing a sink in Python.LoggerTestimplementing a sink in Java andpassing it to
Odr.open.