Skip to content

Commit

Permalink
[typed_data] Remove UnmodifiableXXXView classes
Browse files Browse the repository at this point in the history
In most cases, the (now removed) SDK class was patched so that the constructor redirected to a platform-specific implementation of the unmodifiable view. Uses of the SDK class in the platform code could be rewritten to the more direct use of the implementation class.

The big +/- file changes are from moving the implementation classes from the patch file (typed_data_patch.dart), where they are no longer needed, to the internal file (typed_data.dart).

TEST=ci
Bug: #53785
Change-Id: Iaa7370de25b7fc2d26b24f7733c2892868e593cb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370661
Reviewed-by: Ömer Ağacan <[email protected]>
Reviewed-by: Lasse Nielsen <[email protected]>
Reviewed-by: Siva Annamalai <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>
Reviewed-by: Brian Quinlan <[email protected]>
  • Loading branch information
rakudrama authored and Commit Queue committed Jun 13, 2024
1 parent 7ae9900 commit 18994e6
Show file tree
Hide file tree
Showing 22 changed files with 993 additions and 2,144 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@

[#44876]: https://github.com/dart-lang/sdk/issues/44876

#### `dart:typed_data`

- **BREAKING CHANGE** [#53785][]: The unmodifiable view classes for typed data
have been removed. These classes were deprecated in Dart 3.4.

To create an unmodifiable view of a typed-data object, use the
`asUnmodifiableView()` methods added in Dart 3.3.

This comment has been minimized.

Copy link
@gmpassos

gmpassos Jun 14, 2024

Contributor

FYI: @kevmoo @mit-mit

How do we determine if an instance is unmodifiable without being able to test its hierarchy?

Uint8List bs = ... ;

// With this breaking change, it's impossible to do this:
if (bs is! UnmodifiableUint8ListView) {
  ...
}

This can cause many issues, specially if you need to ensure that some instances is unmodifiable or not.

This comment has been minimized.

Copy link
@gmpassos

gmpassos Jun 14, 2024

Contributor

... we need a isUnmodifiableView

This comment has been minimized.

Copy link
@rakudrama

rakudrama Jun 15, 2024

Author Member

I'm not convinced that we really need this.
Ordinary lists have no way to test for modifiable/unmodifiable.

Usually, having to tell if something is unmodifiable is a sign of an unclear design at a higher level, where an input is confused with an output.

If you feel strongly about this, please open a new issue at https://github.com/dart-lang/sdk/issues/new/choose where we can discuss the use cases

This comment has been minimized.

Copy link
@gmpassos

gmpassos Jun 15, 2024

Contributor

Yes, we need this feature.

"...is a sign of an unclear design at a higher level."
I respectfully disagree, as I believe this is not a well-supported argument.

The Dart collection is designed to be a generic implementation for many use cases. Without the ability to test for an unmodifiable collection, the only way to ensure an unmodifiable parameter is to copy it. The best use case for an unmodifiable parameter is knowing that you don't need to copy or "isolate" it.


[#53785]: https://github.com/dart-lang/sdk/issues/53785

### Tools

#### Linter
Expand Down
4 changes: 2 additions & 2 deletions runtime/vm/dart_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2064,7 +2064,7 @@ TEST_CASE(DartAPI_UnmodifiableTypedDataViewListIsTypedData) {
"import 'dart:typed_data';\n"
"List testMain(int size) {\n"
" var a = new Int8List(size);\n"
" var view = new UnmodifiableInt8ListView(a);\n"
" var view = a.asUnmodifiableView();\n"
" return view;\n"
"}\n";
// Create a test library and Load up a test script in it.
Expand Down Expand Up @@ -2709,7 +2709,7 @@ static void TestUnmodifiableTypedDataViewDirectAccess() {
" for (var i = 0; i < 100; i++) {"
" list[i] = i;"
" }"
" return new UnmodifiableInt8ListView(list);"
" return list.asUnmodifiableView();"
"}\n";
// Create a test library and Load up a test script in it.
Dart_Handle lib = TestCase::LoadTestScript(kScriptChars, nullptr);
Expand Down
Loading

0 comments on commit 18994e6

Please sign in to comment.