rust-lang / log

Logging implementation for Rust
https://docs.rs/log
Apache License 2.0
2.2k stars 254 forks source link

The `*_time` macros are broken on wasm target, maybe disable them or give them a custom implementation? #499

Closed StyMaar closed 2 years ago

StyMaar commented 2 years ago

Today, if you're using the log crate on wasm, it will work fine, until at one point at runtime it panics at the point where one of the dependency uses debug_time! or any of the other *_time! macros, because Instant::now doesn't work in wasm.

panicked at 'time not implemented on this platform', library/std/src/sys/wasm/../unsupported/time.rs:13:9

Because it's a runtime crash, it can be difficult to reproduce and given how hard it is to debug a panic in WASM, it leads to a super frustrating experience.

Disabling these macros in wasm would at least make it a compile error, making it much easier to pinpoint where the problem comes from.

However it would be breaking change, since code that compiles fine today (and maybe crashes at runtime, or maybe not if you're lucky enough not to trigger the right condition) would stop compiling.

The alternative would involve writing a different implementation of MeasureTime for wasm: either by making them do exactly what the “non-time” macros do, or implementing the time measurement in a way that works for WASM (I'm not sure how possible it is in a general way, doing so for the browser is easy but what about non-browser wasm runtimes?)

sfackler commented 2 years ago

debug_time! is not defined by this crate as far as I know.

StyMaar commented 2 years ago

Oops.

Since the bug appeared when I enabled logs, and because the culprit crate (measure_time that is) also expose the log macros itself, I was certain that I was in the log crate when I found the issue with Rust-analyzer's “go to definition”. I guess that's what happens when you keep debugging when you should have gone to bed much earlier.

Sorry for the inconvenience.