search

japaric / ufmt

a smaller, faster and panic-free alternative to core::fmt
Apache License 2.0
352 stars 40 forks source link

str implement uDebug #52

Open andrewgazelka opened 1 year ago

andrewgazelka commented 1 year ago

Where is the panicking branch? I cannot find it. https://github.com/andrewgazelka/ufmt/blob/e97ce1a86cfcc9fa36ffab6ee62762c00e4b3fc9/src/impls/core.rs#L62-L93

bjorn3 commented 1 year ago

I think the panic is somewhere inside c.escape_debug() but I am not quite sure. I did confirm that there is a panic in the codegened code though: https://rust.godbolt.org/z/s74qsE5Yd

andrewgazelka commented 1 year ago

I wouldn't be surprised if it originates from skip_search (is_grapheme_extended in escape_debug_ext which is used in escape_debug)

andrewgazelka commented 1 year ago

would it be all too bad to make uDebug for str equivalent to uDisplay but with quotes? is_grapheme_extended seems to add a bit of overhead anyway.

reitermarkus commented 8 months ago

I just took a look, the panic comes from EscapeDebug::backslash, which uses EscapeIterInner::from_array, which uses slice::copy_from_slice, which may panic.

Also, EscapeIterInner::new uses debug_assert!, which may panic.

reitermarkus commented 6 months ago

It seems my deduction was wrong, and it is caused by EscapeDebug::size_hint not being optimized out, as explained in https://github.com/rust-lang/rust/pull/121805#issuecomment-2055992933.

I have opened https://github.com/rust-lang/rust/pull/124307, which hopefully fixes this.

reitermarkus commented 2 months ago

With https://github.com/rust-lang/rust/pull/124307 and https://github.com/rust-lang/rust/pull/124905 merged, this should now not be panicking anymore on nightly.

reitermarkus commented 2 months ago

From the looks of it, also on 1.80+.

xangelix commented 2 months ago

With rust-lang/rust#124307 and rust-lang/rust#124905 merged, this should now not be panicking anymore on nightly.

Super exciting! Is there anything else left blocking a uDebug implementation for str?