JuliaFolds / Transducers.jl

Efficient transducers for Julia
https://juliafolds.github.io/Transducers.jl/dev/
MIT License
433 stars 24 forks source link

Fix `Partition()` w/ `size` greater than the length of the input #529

Closed LebedevRI closed 1 year ago

LebedevRI commented 2 years ago

complete() only worked correctly if at least a single full partition has been produced already, otherwise the buf would be smaller than the xform(rf).size, and we get buffer overflow, etc.

I've added sufficient test coverage for the issue, that now passes.

I have stumbled into this while trying to write a reduction tree, not sure if this is something that might be interesting here?

Fixes https://github.com/JuliaFolds/Transducers.jl/issues/528

codecov[bot] commented 2 years ago

Codecov Report

Merging #529 (2bf2e55) into master (c8fa4f2) will increase coverage by 0.09%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #529      +/-   ##
==========================================
+ Coverage   95.33%   95.43%   +0.09%     
==========================================
  Files          32       32              
  Lines        2230     2232       +2     
==========================================
+ Hits         2126     2130       +4     
+ Misses        104      102       -2     
Flag Coverage Δ
Pkg.test 94.55% <100.00%> (+0.09%) :arrow_up:
Run.test 95.20% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/library.jl 98.44% <100.00%> (+<0.01%) :arrow_up:
src/nondeterministic_threading.jl 90.81% <0.00%> (+2.04%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

MasonProtter commented 1 year ago

Hey @LebedevRI I’ve finally got the test suite working. Could you rebase on Master? That should get all tests passing and then I can merge.

LebedevRI commented 1 year ago

I've rebased, but did nothing other than that. I don't know if this still works, or needs further changes.

MasonProtter commented 1 year ago

Great, thanks @LebedevRI!

LebedevRI commented 1 year ago

Thank you!