search

ruby / stringio

Pseudo `IO` class from/to `String`.
BSD 2-Clause "Simplified" License
36 stars 26 forks source link

Eagerly defrost chilled strings #94

Closed etiennebarrie closed 7 months ago

etiennebarrie commented 7 months ago

[Bug #20390]

Depends on https://github.com/ruby/ruby/pull/10355

etiennebarrie commented 7 months ago

I updated this to not use have_func following the feedback and changes in https://github.com/ruby/ruby/pull/10355#discussion_r1538423056.

I also realized there were warnings in the output, so I added assertions to ensure they're here and to remove them from the test output.

byroot commented 7 months ago

CI is still using a ruby-head from a couple days ago: ruby 3.4.0dev (2024-03-24T15:27:00Z :detached: 5e4b4d6674), but it's passing on my machine.

eregon commented 7 months ago

CI is still using a ruby-head from a couple days ago

I scheduled a new build. The last one failed due to assertion errors (already fixed on master).

byroot commented 7 months ago

The windows build is still lagging behind by just a handful of commits, but other than that it's now all green.

Earlopain commented 5 months ago

I think this can be reverted now? https://github.com/ruby/ruby/pull/10849

eregon commented 5 months ago

Indeed, I think so and maybe rb_str_chilled_p() can even be removed? cc @etiennebarrie @byroot @casperisfine

casperisfine commented 5 months ago

rb_str_chilled_p() can even be removed?

I don't think so, because of rb_check_frozen(). Unless we stop making it a macro (which maybe we should).

But yeah, I need to look more in details how the last change impact StringIO, we may still want to trigger the warning early to be honest. Not 100% sure.

eregon commented 5 months ago

I don't think so, because of rb_check_frozen(). Unless we stop making it a macro (which maybe we should).

Right, I mean making it non-public API, for example with a rbimpl_ prefix, or indeed not declaring it at all in public headers but then that means changing rb_check_frozen() to a function.