Skip to content

Obtain an immutable array from a mutable one #8826

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

Merged
merged 1 commit into from
Apr 28, 2020
Merged

Conversation

rjolly
Copy link
Contributor

@rjolly rjolly commented Apr 28, 2020

No description provided.

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@@ -268,6 +268,9 @@ object IArray {
// A convenience to avoid having to cast everything by hand
private given [A] as Conversion[Array[A], IArray[A]] = identity[Sub[A]]

/** Obtain an immutable array from a mutable one. */
def apply[T](s: Array[T]): IArray[T] = s
Copy link
Member

Choose a reason for hiding this comment

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

This is unsafe, so it should be called something like unsafeFromArray

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM with this more complete description

@@ -268,6 +268,9 @@ object IArray {
// A convenience to avoid having to cast everything by hand
private given [A] as Conversion[Array[A], IArray[A]] = identity[Sub[A]]

/** Obtain an immutable array from a mutable one. */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/** Obtain an immutable array from a mutable one. */
/** Convert an array into an immutable array without copying, the original array
* must _not_ be mutated after this or the guaranteed immutablity of IArray will
* be violated.
*/

@smarter smarter merged commit 96e45a7 into scala:master Apr 28, 2020
@sjrd
Copy link
Member

sjrd commented Apr 28, 2020

I don't think we should have this at all. We don't have List.unsafeFromListBuffer, so why should we have IArray.unsafeFromArray?

@smarter
Copy link
Member

smarter commented Apr 28, 2020

But we do have ArraySeq.unsafeWrapArray. I think it makes sense in the context of Java interop where you're getting arrays from Java which are documented as immutable, in the same way that all values coming from Java are nullable but we still provide .nn under -Yexplicit-nulls.

@rjolly
Copy link
Contributor Author

rjolly commented Apr 28, 2020

List.unsafeFromListBuffer would be useful to me. In Java we have Collections.unmodifiableCollection(c) and friends.

@sjrd
Copy link
Member

sjrd commented Apr 29, 2020

Java never encodes immutability in the type system, so unmodifiableX is not an unsafe operation. It's using j.u.List that's unsafe to start with.

rjolly added a commit to rjolly/scas that referenced this pull request Nov 11, 2020
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.

4 participants