Add Log implementation for &impl Log and Arc<impl Log>

[piegamesde]

Closes #458. Turns out that Rc<Log> isn't possible because Log: Sync. Other candidates for adding:

• Cow
• Option
• () (this would allow us to get rid of the internal NopLogger)
• Pin (?)
• Probably forgot something

[KodrAus]

Thanks @piegamesde! I think this is a great starting point. I think ideally we'd stick to implementations that can unconditionally forward to another implementation, like Box<T>, Arc<T>, &T. Just to avoid any possible confusion over how something like Option<T> or () should actually behave.

[piegamesde]

I'd argue that the intent for Option<T> is rather clear and for () more or less as well. But I agree that this can be overdone (looking at you, Haskell) and one has to draw the line somewhere.

This still leaves Pin and Cow open. I'd be in favor of Pin because I don't see any reason not to (the first person coming up with a legitimate use case for Pin<Box<Log>> wins a cookie). For Cow, I don't know. Technically the implementation would simply forward along its Borrow so its good?

[KodrAus]

Yeh, there are subtleties in the semantics that I don't think the benefit outweighs for Option or (). Pin and Cow both do seem reasonable. Would you like to include them in this PR or follow-up with another?

[piegamesde]

Would you like to include them in this PR or follow-up with another?

Let's do all that make sense right now. I added Pin, but I had to back off Cow because it's Clone constraint on T. This is something that does make little sense for loggers. This is not the first time I've wished for some MaybeOwned in Rust, i.e. a Cow without the "clone" and the "write" part.

For the potential recursion issue mentioned in review, I added #![deny(unconditional_recursion)] instead of a test.

[KodrAus]

Ah it looks like we might need to back Pin out too 🙂 It was actually still unstable in 1.31.0

[piegamesde]

Yes, I've seen the CI fail too. Is there a way to feature guard this only for newer Rust versions? Edit: found it: https://github.com/dtolnay/rustversion#use-cases

[KodrAus]

Ah unfortunately we can’t really introduce any unconditional dependencies such as rustversion to log. We could do some version detection in the build script, but the best option for now might be to simply roll back Pin.

[piegamesde]

I removed the Pin implementation in a second commit so that you can simply revert it after the next MSRV bump. Feel free to squash it if you prefer it that way.

[KodrAus]

Thanks for working on this @piegamesde!