Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[WIP] Don't merge: Compile with Dart #241

Closed
wants to merge 1 commit into from

Conversation

alfonsogarciacaro
Copy link
Contributor

Some changes necessary to target Dart in the current state of Fable Snake Island compiler.

@@ -3,15 +3,15 @@ open System

[<Struct>]
type internal RingState<'item> =
| Writable of wx:'item array * ix:int
| ReadWritable of rw:'item array * wix:int * rix:int
| Writable of wx:'item option array * ix:int
Copy link
Member

Choose a reason for hiding this comment

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

I understand the impulse to optionize, but the array is internal impl detail and there's nothing wrong with it using unchecked default for values because index range is tightly controlled anyway. From performance perspective having an option instead of value doubles the number of allocations and GC pressure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is a tricky thing with the Dart implementation right now. Dart has null safety so if I try to assign null to a generic it will complain and there's no such a thing as default(T) so I haven't found a way to allocate an array without providing a fill value.

At the moment, I'm compiling options as nullables in Dart so by using 'item option I can assign null as default value. But I understand this has implications for .NET (would OptionValue help?) so I'm not asking to change it. Let's see if I manage to solve the issue in Dart, if not maybe we can also use #if FABLE_COMPILER here, although I'm also trying to avoid proliferation of conditional compilation :)

Copy link
Member

Choose a reason for hiding this comment

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

yes, voption should work and I think would be more or less free from perf/cost perspective.

@et1975
Copy link
Member

et1975 commented Jun 13, 2024

If someone wants to pickup this work we can reopen.

@et1975 et1975 closed this Jun 13, 2024
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.

None yet

2 participants