initial draft for downcastable Log

djugei commented

2026-09-19 03:25:02 -05:00

(Migrated from github.com)

fixes #666.
i do not expect this to be merged as is, this is more of a discussion helper. Names are up for bikeshedding.
technically this is not a breaking change i think, as anything that implements log automatically implements LogAny though i agree that it looks weird.

can't just have a method on Log sadly as that would introduce a Sized requirement (for some reason). Can't add the Any bound to Log itself as it would need to be 'static

i have tested this with indicatif-log-bridge and env_logger and it works

fixes #666. i do not expect this to be merged as is, this is more of a discussion helper. Names are up for bikeshedding. technically this is not a breaking change i think, as anything that implements log automatically implements LogAny though i agree that it looks weird. can't just have a method on Log sadly as that would introduce a `Sized` requirement (for some reason). Can't add the Any bound to Log itself as it would need to be 'static i have tested this with indicatif-log-bridge and env_logger and it works

KodrAus commented

2026-09-21 07:09:52 -05:00

(Migrated from github.com)

Thanks for the PR @djugei! I think we can get away without the as_any method, since recent Rust releases support dyn trait upcasting. Maybe something like:

pub trait GlobalLog: Log + Any {}

impl<T: Log + Any> GlobalLog for T {}

I did look at a few alternative designs that try to fully hide the downcasting support, but they all had unsatisfying trade-offs since we accept a Box<dyn Trait> in our public API.

Thanks for the PR @djugei! I think we can get away without the `as_any` method, since recent Rust releases support dyn trait upcasting. Maybe something like: ```rust pub trait GlobalLog: Log + Any {} impl<T: Log + Any> GlobalLog for T {} ``` I did look at a few alternative designs that try to fully hide the downcasting support, but they all had unsatisfying trade-offs since we accept a `Box<dyn Trait>` in our public API.

djugei commented

2026-09-21 10:55:31 -05:00

(Migrated from github.com)

just makes user code a bit uglier
log::logger().as_any().downcast_ref()
(log::logger() as &'static dyn std::any::Any).downcast_ref()
but yes, no additional capabilities so can be removed.

is this feature in general something the project would like to add?

just makes user code a bit uglier `log::logger().as_any().downcast_ref()` `(log::logger() as &'static dyn std::any::Any).downcast_ref()` but yes, no additional capabilities so can be removed. is this feature in general something the project would like to add?

KodrAus commented

2026-09-21 17:39:14 -05:00

(Migrated from github.com)

I'd be in favour of this approach with the as_any method removed. I think we should call the trait GlobalLog, and use it as our marker for a Log that can be plugged into the shared slot.

I'd be in favour of this approach with the `as_any` method removed. I think we should call the trait `GlobalLog`, and use it as our marker for a `Log` that can be plugged into the shared slot.

Thomasdezeeuw commented

2026-09-22 03:22:53 -05:00

(Migrated from github.com)

I think this a breaking change. We didn't the std::any::Any trait bound on these functions before, now we effectively do. So we might as well do pub trait Log: std::any::Any { ... }.

I think this a breaking change. We didn't the `std::any::Any` trait bound on these functions before, now we effectively do. So we might as well do `pub trait Log: std::any::Any { ... }`.

KodrAus commented

2026-09-22 03:30:53 -05:00

(Migrated from github.com)

@Thomasdezeeuw Yeh I checked this as well, and couldn’t actually trigger a compile error by flipping from Box<dyn Log> to Box<dyn GlobalLog>, I believe because of the default 'static object bound. If there’s any chance this is a breaking change though we should avoid it.

Adding an Any bound to Log itself is almost certainly a breaking change.

@Thomasdezeeuw Yeh I checked this as well, and couldn’t actually trigger a compile error by flipping from `Box<dyn Log>` to `Box<dyn GlobalLog>`, I believe because of the default `'static` object bound. If there’s any chance this is a breaking change though we should avoid it. Adding an `Any` bound to `Log` itself is almost certainly a breaking change.

Thomasdezeeuw commented

2026-09-22 03:36:36 -05:00

(Migrated from github.com)

Not 100% accurate, but I think https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=7e1712bc6dbdeb86780c85de4b0a7e6c shows it's a problem.

djugei commented

2026-09-22 03:42:26 -05:00

(Migrated from github.com)

i tried doing Log: Any but that lead to lifetime issues with impl Log for &'_ T: Log so i did a separate trait, though i guess those could be tightened to &'static and it would be fine.

i think any concrete type that implements Log will also implement Any, but if you have a function that bounds only on Log then the compiler is ignorant of that fact, so yeah i guess this is a breaking change after all, though one that can be fixed with minimal changes/no structural changes.

i tried doing `Log: Any` but that lead to lifetime issues with `impl Log for &'_ T: Log` so i did a separate trait, though i guess those could be tightened to &'static and it would be fine. i think any concrete type that implements Log will also implement Any, but if you have a function that bounds only on Log then the compiler is ignorant of that fact, so yeah i guess this is a breaking change after all, though one that can be fixed with minimal changes/no structural changes.

KodrAus commented

2026-09-22 03:44:54 -05:00

(Migrated from github.com)

Ah yes I see. If you explicitly already have a Box<dyn Log> you can’t pass it to the set_boxed_logger function when we introduce GlobalLog.

That would mean we’d have to introduce a new set of APIs for this if we wanted to do it and deprecate the old ones. I’m not sure that level of churn is worth it.

Ah yes I see. If you explicitly already have a `Box<dyn Log>` you can’t pass it to the `set_boxed_logger` function when we introduce `GlobalLog`. That would mean we’d have to introduce a new set of APIs for this if we wanted to do it and deprecate the old ones. I’m not sure that level of churn is worth it.

djugei commented

2026-09-22 03:57:47 -05:00

(Migrated from github.com)

That would mean we’d have to introduce a new set of APIs for this if we wanted to do it and deprecate the old ones. I’m not sure that level of churn is worth it.

it is even worse than that: since we need to store the logger we have to decide on a concrete type, so those would be 2 completely separate apis, which does not make a lot of sense to me, as it would essentially be keeping two versions. in that case i would just keep one version in the library and let cargo do its job for versioning.

alternatively we can try to think about &'static Log always being Any a bit harder and maybe do an unsafe cast if we can convince ourselves that it is always safe? i sadly do not know a ton about trait objects.

then again this functionality is achievable without using Any at all (see issue) but uses unsafe to do so in the first place. i was hoping for an elegant solution with Any.

> That would mean we’d have to introduce a new set of APIs for this if we wanted to do it and deprecate the old ones. I’m not sure that level of churn is worth it. it is even worse than that: since we need to store the logger we have to decide on a concrete type, so those would be 2 completely separate apis, which does not make a lot of sense to me, as it would essentially be keeping two versions. in that case i would just keep one version in the library and let cargo do its job for versioning. alternatively we can try to think about &'static Log always being Any a bit harder and maybe do an unsafe cast if we can convince ourselves that it is always safe? i sadly do not know a ton about trait objects. then again this functionality is achievable without using Any at all (see issue) but uses unsafe to do so in the first place. i was hoping for an elegant solution with Any.

This pull request is broken due to missing fork information.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u origin 'djugei/dyn_downcast:djugei/dyn_downcast'

git switch 'djugei/dyn_downcast'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'master'

git merge --no-ff 'djugei/dyn_downcast'

git switch 'djugei/dyn_downcast'

git rebase 'master'

git switch 'master'

git merge --ff-only 'djugei/dyn_downcast'

git switch 'djugei/dyn_downcast'

git rebase 'master'

git switch 'master'

git merge --no-ff 'djugei/dyn_downcast'

git switch 'master'

git merge --squash 'djugei/dyn_downcast'

git switch 'master'

git merge --ff-only 'djugei/dyn_downcast'

git switch 'master'

git merge 'djugei/dyn_downcast'

git push origin 'master'

Rows
Columns

initial draft for downcastable Log #739

Checkout

Merge