Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
adri326 opened this issue May 22, 2024 · 2 comments
Open

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

adri326 opened this issue May 22, 2024 · 2 comments

Comments

@adri326
Copy link

adri326 commented May 22, 2024

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
Copy link
Collaborator

dpc commented May 22, 2024

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
Copy link
Member

Both of these changes make sence.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants