slog-rs / slog

Structured, contextual, extensible, composable logging for Rust
https://slog.rs/
Apache License 2.0
1.58k stars 95 forks source link

`Logger::root_typed` requires doc_hidden elements to be passed around #334

Open adri326 opened 6 months ago

adri326 commented 6 months ago

I would like to use slog inside of a bootloader, meaning that I need to be able to turn off logging when necessary to save binary size, and I also want to avoid relying on static memory (and slog seems to be the only logging library that I have found that does not have this baked in).

However, I fear that if I am to use the default Logger<Arc<SendSyncRefUnwindSafeDrain>>, then some optimizations will be missed, especially when I want to fully turn off logging.

Trying to use Logger::root_typed, I quickly came to an issue: it's impossible for a function to give the correct trait requirements on the drain for Logger to be used:

fn main() {
    let drain = obtain_drain();
    let logger = Logger::root_typed(&drain, slog::o!());

    my_function(&logger);
}

// Unfortunately not sufficient:
fn my_function<D: SendSyncUnwindDrain>(logger: &Logger<D>) {
    slog::info!(logger, "Hello world");
}

The only way around this that I have found so far is to require the generic parameter D to implement slog::SendSyncUnwindDrain<Ok = (), Err = slog::Never>, which requires the use of the doc_hidden bottom type placeholder.

I think a very simple solution would be to provide a trait, Log, which would be implemented by Logger when the requirements for Logger::log are satisfied. With it, the code above would simplify to fn my_function(logger: &impl slog::Log). If the method on Log is called something other than log (say log_record), then such a change would only require a minor version bump, and Logger::log could be left as-is, with the macros calling Log::log_record directly (assuming that until now, slog::log! could only be used with a Logger).

dpc commented 6 months ago

I guess that kind of works, maybe? The syntax on the macros needs to be so that that importing new trait is not necessary, but I could see &impl log::Log being useful for other thing than what's described here, like testing.

BTW. IIRC slog::Never is #[doc_hidden], just to leave the door open to update it to ! when it's ready. But it's been many years and there's already https://doc.rust-lang.org/std/convert/enum.Infallible.html , which is supposed to perform the same function, so maybe we could switch to that already?

Techcable commented 2 months ago

Both of these changes make sence.

A Log trait alias and changing slog::Never to std::convert::Infailable are both good ideas.