Skip to content

Strict byteArray of IV c#16

Merged
kazu-yamamoto merged 1 commit into
kazu-yamamoto:masterfrom
TheKK:master
Sep 16, 2023
Merged

Strict byteArray of IV c#16
kazu-yamamoto merged 1 commit into
kazu-yamamoto:masterfrom
TheKK:master

Conversation

@TheKK
Copy link
Copy Markdown

@TheKK TheKK commented Sep 15, 2023

This provides a way to seq IV c and its content when client wants to prevent thunk exploding, It's not rare to apply multiple ivAdd on single IV c and we often don't want thunk here.

About the idea, "using StrictData", in our previous discussion, I found that was a very time-consuming job to do if we want to make sure everything still works after change.

Take Point for instance

data Point curve =

it has instance of NFData but its Integer (x and y) field was not marked as strict, which is difficult be to certain that all its user/algorithm never requires these fields to be lazy. And we have more than one type like this.

To reduce the impact, I'll vote for only changing the part I knew that has issue (thunk) and have confidence in solving it in a safer way.

This provides a way to seq `IV c` and its content when client wants to
prevent thunk exploding, It's not rare to apply multiple `ivAdd` on
single `IV c` and we often don't want thunk here.
@kazu-yamamoto kazu-yamamoto self-requested a review September 16, 2023 03:38
Copy link
Copy Markdown
Owner

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good start.

@kazu-yamamoto
Copy link
Copy Markdown
Owner

General comment. Even with StractData, we can mark suspicious fields with "~".

@kazu-yamamoto kazu-yamamoto merged commit 2ea93fd into kazu-yamamoto:master Sep 16, 2023
@kazu-yamamoto
Copy link
Copy Markdown
Owner

Merged.
Thank you for your contribution!
I will make a new release if you wish.

@TheKK
Copy link
Copy Markdown
Author

TheKK commented Sep 16, 2023

I'm okay with the current version (with simple workaround) so feel free to release new version when you think it's ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants