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

Match concrete size array #1074

Closed
wants to merge 9 commits into from
Closed

Conversation

andreistefanescu
Copy link
Contributor

No description provided.

Copy link
Contributor

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

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

The general idea looks reasonable to me. Most of my comments concern documentation, which would help a lot in making this (fairly nuanced) code easier to understand.

@@ -1629,6 +1632,21 @@ possibleAllocs n mem =
Just (AllocInfo atp sz mut alignment loc) ->
[SomeAlloc atp n sz mut alignment loc]


newtype MemoIO m a = MemoIO (IORef (Either (m a) a))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please leave some comments stating with the conventions are for MemoIO. If I understand correctly, Left indicates a suspended computation and Right indicates an evaluated result, but it would be helpful to state this explicitly.

@@ -527,15 +528,16 @@ multiUnion :: (Ord k, Semigroup a) => Map k a -> Map k a -> Map k a
multiUnion = Map.unionWith (<>)

writeSourceSize ::
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only ever return pure Nothing if the WriteSource corresponds to a byte array of unbounded size, correct? It would be worth adding some Haddocks stating as much.

(This isn't your fault, as this function existed in Haddock-less form before this patch, but it would be worth addressing this now that we export this function in the public API.)

@andreistefanescu andreistefanescu force-pushed the match-concrete-size-array branch 4 times, most recently from adb26d8 to 4943522 Compare June 28, 2023 00:32
@RyanGlScott RyanGlScott force-pushed the match-concrete-size-array branch from 470cb28 to 5c2c29c Compare January 26, 2024 19:42
@RyanGlScott RyanGlScott marked this pull request as draft March 1, 2024 16:43
@RyanGlScott
Copy link
Contributor

Superseded by #1178.

@RyanGlScott RyanGlScott deleted the match-concrete-size-array branch May 14, 2024 17:40
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