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.

Sign up for GitHub

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

Jump to bottom

BytesBuilder.takeBytes should explicitly provide a Uint8List #27818

Closed
Hixie opened this issue Nov 14, 2016 · 6 comments
Closed

BytesBuilder.takeBytes should explicitly provide a Uint8List #27818

Hixie opened this issue Nov 14, 2016 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. customer-flutter library-io type-enhancement A request for a change that isn't a bug

Comments

@Hixie
Copy link
Contributor

Hixie commented Nov 14, 2016

...so that it can be statically proved that you can obtain a ByteBuffer from the result. Flutter is going to be relying on this all over the place. For example, we're going to rely on File.readAsBytes providing a Uint8List. We already rely on the UTF8 decoder actually returning a Uint8List.
@floitschG floitschG added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels Nov 15, 2016
@floitschG
Copy link
Contributor

This is extremely dangerous. None of these functions promise a Uint8List. It could easily be that they return a const <int>[] for some empty list here or there.

@whesse
Copy link
Contributor

whesse commented Nov 15, 2016

It is unclear from your reply whether you mean the proposed change is dangerous, or that the status quo is dangerous, and should be changed so the proposed change is safe. Could you clarify, and suggest a resolution?

@lrnh

@Hixie
Copy link
Contributor Author

Hixie commented Nov 15, 2016

We agree that our current code is dangerous, but we don't really have a choice. This is performance-critical code. That's why we'd like the APIs changed to be explicitly always returning raw buffers. It should be a backwards-compatible change.

@zanderso zanderso added type-enhancement A request for a change that isn't a bug customer-flutter labels Apr 7, 2017
@Hixie Hixie mentioned this issue Dec 30, 2018
Closed
@tvolkert
Copy link
Contributor

tvolkert commented May 8, 2019

https://dart-review.googlesource.com/c/sdk/+/101742

@tvolkert tvolkert mentioned this issue May 8, 2019
Closed
@tvolkert
Copy link
Contributor

tvolkert commented May 8, 2019

Breaking change request: #36900

dart-bot pushed a commit that referenced this issue Jun 20, 2019
@tvolkert
Declare return types of Uint8List.
9173aed 
These methods all were returning Uint8List, yet they were only
declared to return List<int>. This forced callers to either defensively
wrap the return values in Uint8List, or to assume the contravariant
return value:

* Utf8Codec.encode()
* BytesBuilder.takeBytes()
* BytesBuilder.toBytes()
* File.readAsBytes()
* File.readAsBytesSync()
* RandomAccessFile.read()
* RandomAccessFile.readSync()
* Uint8List.sublist()

Since it's related, this change also updates the following sublist()
methods to declare that they return the a sublist of the same type as
the source list:

* Int8List
* Uint8ClampedList
* Int16List
* Uint16List
* Int32List
* Uint32List
* Int64List
* Uint64List
* Float32List
* Float64List
* Float32x4List
* Int32x4List
* Float64x2List

Bug: #36900
Bug: #31547
Bug: #27818
Bug: #35521
Change-Id: Ic3bc1db0d64de36fb68b1d8d98037eed1464f978
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/101742
Commit-Queue: Todd Volkert <tvolkert@google.com>
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
@tvolkert
Copy link
Contributor

This is implemented now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. customer-flutter library-io type-enhancement A request for a change that isn't a bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Hixie @whesse @zanderso @floitschG @tvolkert