acton icon indicating copy to clipboard operation
acton copied to clipboard

sum start argument is optional and has no default, leading to segfault

Open plajjan opened this issue 1 year ago • 4 comments

Acton Version

0.21.0

Steps to Reproduce and Observed Behavior

Our builtin sum() function takes an optional start argument, like so:

def sum [A(Plus)] (iter: Iterable[A], start: ?A) -> A:

but in the C code, it relies on start being not NULL (or it'll segfault). I presume start should always be set but I don't quite understand why start is a ?A and not just an int with a default of 0?

Expected Behavior

Not segfault

plajjan avatar Apr 02 '24 20:04 plajjan

@sydow I guess you might know what the intention was here? :)

plajjan avatar Apr 02 '24 20:04 plajjan

Hi,

this is, I think, a result of unfinished thinking several years ago.

To give a default value of 0 to start would restrict the type of sum to

[A(Number)] => (iter: Iterable[A], start: ?A) -> A

(with Number replacing Plus), so the function could not be used to e.g. concatenate lists.

I vaguely remember discussing adding a unit element to protocol Plus to make it a monoid and have that be the default value, but this went nowhere. So, maybe the best thing now is to do as you propose. I guess it is also about time to actually implement the function in Acton and not have it be NotImplemented.

Björn

On 2 Apr 2024, at 22:26, Kristian Larsson @.***> wrote:

Acton Version 0.21.0 Steps to Reproduce and Observed Behavior Our builtin sum() function takes an optional start argument, like so: def sum [A(Plus)] (iter: Iterable[A], start: ?A) -> A:

but in the C code, it relies on start being not NULL (or it'll segfault). I presume start should always be set but I don't quite understand why start is a ?A and not just an int with a default of 0? Expected Behavior Not segfault — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

sydow avatar Apr 03 '24 07:04 sydow

@sydow I guess an alternative to changing to default of 0 would be to remove the optional bit, so the user has to specify the start value explicitly instead!?

plajjan avatar Apr 03 '24 13:04 plajjan

Yes, of course that’s an alternative, but it seems tedious to have to say 0, which I would guess is by far the most common case, every time.

On 3 Apr 2024, at 15:52, Kristian Larsson @.***> wrote:

@sydow I guess an alternative to changing to default of 0 would be to remove the optional bit, so the user has to specify the start value explicitly instead!? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

sydow avatar Apr 03 '24 14:04 sydow