Transducers.jl icon indicating copy to clipboard operation
Transducers.jl copied to clipboard

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

Open LebedevRI opened this issue 3 years ago • 1 comments

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

LebedevRI avatar Jun 23 '22 22:06 LebedevRI

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.

codecov[bot] avatar Jun 23 '22 22:06 codecov[bot]

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.

MasonProtter avatar Nov 25 '22 18:11 MasonProtter

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

LebedevRI avatar Nov 25 '22 19:11 LebedevRI

Great, thanks @LebedevRI!

MasonProtter avatar Nov 25 '22 22:11 MasonProtter

Thank you!

LebedevRI avatar Nov 25 '22 23:11 LebedevRI