matklad / once_cell

Rust library for single assignment cells and lazy statics without macros
Apache License 2.0
1.84k stars 110 forks source link

impl Display for Lazy<T: Display> #94

Closed nalply closed 1 year ago

nalply commented 4 years ago

Use case: I have some Lazy<&'static str> fields.

An example inside a macro from a project:

pub const DESCRIPTION: &'static str = $description;

pub const VALIDATION_REGEX: Lazy<&'static str> = Lazy::new(
  || Box::leak(trim_1_1(&rx::$rx.as_str()).into_boxed_str()));

Then format!("validation failed, must match {}", Docid::VALIDATION_REGEX) fails because Lazy<&'static str> doesn't implement Display. To get this to work, use &*Docid::VALIDATION_REGEX instead or Lazy::force().

It might be surprising for people that DESCRIPTION and VALIDATION_REGEX are different, and the solution with &* is possibly not very self-evident.

That's why I propose here: Lazy could implement Display if the value implements Display.

An open question is whether Lazy could implement other traits, too, as it has been done for unsync::OnceCellwith Clone (https://github.com/matklad/once_cell/issues/4).

jhpratt commented 4 years ago

@matklad Would a PR implementing this be accepted? I'm also interested in implementing impl<T, U> AsRef<U> for Lazy<T> where T: AsRef<U> for similar reasons.

matklad commented 4 years ago

I am really torn about this...

On the one hand, forcing in display and as_ref seems surprising.

On the other hand, Lazy is really just a convenience type, it does implicit forcing in deref

On the third hand, it seems like this would be a relatively minor convenience, and, where you need it, it's not too hard to newtype lazy

On the fourth hand, I am pretty sure that we'd go with concervative option for std, so, once std-lazy rfc is implemented, it makes sense to be expand once_cell API considerably, just to try more stuff out outside of std.

briansmith commented 4 years ago

This would turn into a never ending stream of requests to forward one trait after another. It seems like a better solution would be to solve the problem for deref coersions in general at the language/core level.

jhpratt commented 4 years ago

Yeah, that's definitely true. Perhaps waiting for this to get into std would be best. I just ran into AsRef because I was iterating over a reference, which led to a triple dereference, something I'd never had to do before.

If there were some way to automatically implement all traits implemented by the inner type, that would be ideal. But I'm fairly certain that's neither possible nor desired in the compiler.

nalply commented 4 years ago

Wow, a triple dereference. Can you show the code?

jhpratt commented 4 years ago

(Un?)fortunately I already rewrote that bit of code to avoid it. One of the dereferences was due to the iterator, the second was because I was iterating over references (to avoid taking ownership), and the third was followed by a reference (equivalent to Lazy::force). I certainly wouldn't say it was unidiomatic, but rather a side effect of how I was passing data around.

matklad commented 1 year ago

Thinking about this more, I am inclined to close this issue. It's important that operatiof of forcing a Lazy is visible in the source code. I recall debugging several performance issues in my IntelliJ day which boiled down to "stuff is printing to logs, and printing forces lazy evaluation".