Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Seq.sum returns incorrect result for types that define the get_Zero operator #1284

Closed
cata opened this issue Nov 2, 2022 · 5 comments
Closed
Assignees

Comments

@cata
Copy link
Contributor

cata commented Nov 2, 2022

Please see this Try WebSharper snippet for the repro.

@Jand42
Copy link
Member

Jand42 commented Nov 3, 2022

@cata Thanks, indeed Seq.sum and some others probably just have a single implementation for the default + operator. I have fixed the related issue #1283 first, as it's needed to process resolving generic operators, looking into this next.

@cata
Copy link
Contributor Author

cata commented Nov 3, 2022

@Jand42 - awesome, thank you!

@Jand42
Copy link
Member

Jand42 commented Nov 3, 2022

These are the Seq/List/Array functions that I would upgrade for better generic handling: sum, sumBy, average, averageBy.

Functions using comparisons (sorts) are already working well enough as the actual comparison is handled by a helper that checks for all cases. Similar idea could be implemented for required operations here. Possibly with fallback to a flat simple implementation with direct operator uses, for runtime performance.

@Jand42
Copy link
Member

Jand42 commented Nov 4, 2022

@cata This is in GH build 6.0.5.244 right now.

Primitive numeral types still default to a function call that uses plain JS operators inside, but any other type compile to an inline that uses the type's defined get_Zero and op_Addition members. (And also DivideByInt for average)

Breaking change: you should now use sum, sumBy, average, averageBy functions with resolved generics, or annotate with [<Inline>]

@cata
Copy link
Contributor Author

cata commented Nov 4, 2022

@Jand42 - super, you guys are awesome!

@Jand42 Jand42 closed this as completed Dec 15, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants