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

Add "Unsafe" modules #1026

Open
treeowl opened this issue Aug 13, 2024 · 6 comments
Open

Add "Unsafe" modules #1026

treeowl opened this issue Aug 13, 2024 · 6 comments

Comments

@treeowl
Copy link
Contributor

treeowl commented Aug 13, 2024

The API currently exposes a few functions that break the abstraction barrier and sometimes allow the construction of invalid structures. We should collect those in separate modules, with the hope of eventually removing them from the "safe" interface.

@meooow25
Copy link
Contributor

I know I suggested this in #1025 (comment), but it needs more thought 😄

What would be the distinction between Map.Internal and Map.Unsafe in this case?

@treeowl
Copy link
Contributor Author

treeowl commented Aug 14, 2024

I think of Unsafe as having much stronger stability expectations than Internal.

@meooow25
Copy link
Contributor

That sounds fine in theory, but in practice I haven't seen Internal modules being unstable at all (at least since I have been paying attention to it).

I'm just trying to see if we can avoid having too many modules.

Alternative option 1: We make Internal stable (and it follows PVP) and move unsafe functions there.

Alternative option 2 (more drastic): We add Unsafe but hide Internal. Unsafe is stable. The exports in the public module and the exports in the Unsafe module are disjoint and together make up the Internal module.

@meooow25
Copy link
Contributor

To get a sense of things,

List of functions in Data.Map.Lazy that can violate Map invariants:

  • fromAscList
  • fromAscListWith
  • fromAscListWithKey
  • fromDistinctAscList
  • fromDescList
  • fromDescListWith
  • fromDescListWithKey
  • fromDistinctDescList
  • mergeWithKey
  • mapKeysMonotonic

More functions in Data.Map.Internal that can violate Map invariants:

  • Map constructors
  • WhenMissing constructors
  • WhenMatched constructors
  • bin
  • balance
  • balanceL
  • balanceR
  • insertMax
  • link
  • link2
  • glue

@treeowl
Copy link
Contributor Author

treeowl commented Aug 15, 2024

That sounds fine in theory, but in practice I haven't seen Internal modules being unstable at all (at least since I have been paying attention to it).

You write this just a couple months after changing the internal representations of IntSet and IntMap!

@meooow25
Copy link
Contributor

That's true but it is a rare event 😄
IntMap hasn't changed in ~19 years. IntSet has changed once ~13 years ago (packed tips added).

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

No branches or pull requests

2 participants