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

Proposal: rename Data.ByteArray to Data.Array.Byte #19

Closed
Bodigrim opened this issue Nov 22, 2021 · 12 comments
Closed

Proposal: rename Data.ByteArray to Data.Array.Byte #19

Bodigrim opened this issue Nov 22, 2021 · 12 comments
Labels
approved Approved by CLC vote base-4.17 Implemented in base-4.17 (GHC 9.4)

Comments

@Bodigrim
Copy link
Collaborator

MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6742

Currently unreleased base HEAD contains a new Data.ByteArray module. It's been noticed that this is an unfortunate naming, because it clashes with memory and thus has a potential to break any of its 203 downstream dependencies.

The proposal is to rename it to Data.Array.ByteArray: Data.Array is a well-attested namespace (e. g., in array), and Data.Array.ByteArray is currently unused.

Additionally, the MR adds {-# LANGUAGE Trustworthy #-}, because otherwise deepseq has to resort to pretty nasty hacks to provide instance NFData.

This is unfortunately a rather pressing matter, because deepseq, bytestring and text need to adopt asap. Since the impact on ecosystem is positive (less packages will be broken than with status quo) and GHC MR was open for a long time, I hope this does not require prolonged deliberations.

@chessai
Copy link
Member

chessai commented Nov 22, 2021

Based on the breakage, I say we change the name. I don't like the proposed name, but I don't dislike it either. I don't think we should put too much thought into the name, though.

So I'm +1

@konsumlamm
Copy link
Contributor

What if we eventually want to add other kinds of arrays to base, like normal Arrays (cf Data.Primitive.Array)? Would that module be called Data.Array.Array then (Data.Array is already used by array)? That would sound pretty silly imo. Or will that just never happen?

If PrimMonad should eventually be moved to base, stealing Data.Primitive._ from primitive might work...

@Bodigrim
Copy link
Collaborator Author

@konsumlamm there are currently no plans to move more parts of primitive to base, and I see little incentive to do so. Note that while Data.ByteArray was derived from Data.Primitive.ByteArray, it could have been also based on Data.ByteString.Short or Data.Text.Array. There is nothing primitive-specific, it is just a boxed ByteArray#.

@gwils
Copy link
Contributor

gwils commented Nov 23, 2021

+1, let's avoid that breakage and keep things moving. I find the name to be ugly, but acceptable, and not out of place among base or array.
Consider instead Data.Array.Byte? the -Array suffix is redundant, no?

@Bodigrim
Copy link
Collaborator Author

Data.Array.Byte is also fine with me.
@emilypi @cigsender @cgibbard what do you think? Do you prefer Data.Array.ByteArray or Data.Array.Byte?

@emilypi
Copy link
Member

emilypi commented Nov 24, 2021

That's quite the name clash. Approved.

@mixphix
Copy link
Collaborator

mixphix commented Nov 24, 2021

I'm not a fan of the whole Data.*/Control.* naming scheme in the first place but that's a huge can of worms. I think Data.Array.Byte is a happy medium here.

@cgibbard
Copy link
Contributor

I'm okay with either name, and approve of avoiding the breakage situation. Normally I would slightly lean in the direction of Data.Array.ByteArray, since the module having the type's name in its name helps users identify where the type is coming from in cases where it has been imported unqualified. However, the 'array' package is full of modules like Data.Array.IO which exports IOArray. So Data.Array.Byte would be consistent with that.

@Bodigrim
Copy link
Collaborator Author

@chessai Are you comfortable with Data.Array.Byte?

@chessai
Copy link
Member

chessai commented Nov 24, 2021

I'm fine with it

@Bodigrim
Copy link
Collaborator Author

Cool, seems everyone is on board with Data.Array.Byte. I've updated https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6742. Closing as approved, thanks everyone.

@Bodigrim Bodigrim added the approved Approved by CLC vote label Nov 24, 2021
@Bodigrim Bodigrim changed the title Proposal: rename Data.ByteArray to Data.Array.ByteArray Proposal: rename Data.ByteArray to Data.Array.Byte Nov 25, 2021
@chshersh
Copy link
Member

I'm trying to summarise the state of this proposal as part of my volunteering effort to track the progress of all approved CLC proposals.

Field Value
Author @Bodigrim
Status merged
base version 4.17.0.0
Merge Request (MR) https://gitlab.haskell.org/ghc/ghc/-/merge_requests/6742
Blocked by nothing
CHANGELOG entry present
Migration guide not needed

Please, let me know if you find any mistakes 🙂

@chshersh chshersh added the base-4.17 Implemented in base-4.17 (GHC 9.4) label Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote base-4.17 Implemented in base-4.17 (GHC 9.4)
Projects
None yet
Development

No branches or pull requests

8 participants