Create unsupported bitfield implementation#27390
Create unsupported bitfield implementation#27390jonahwilliams wants to merge 3 commits intoflutter:masterfrom
Conversation
|
test? (yes i'm aware that's going to be a yak shave of epic proportions...) |
|
Good news: plumbing require to do so is working its way through the engine: I also need to confirm this compiles internally and in fuchsia-tree |
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| /// Unsupported. |
There was a problem hiding this comment.
This doc comment is unhelpful.
| /// Unsupported. | ||
| int get kMaxUnsignedSMI => throw UnsupportedError('Not supported in environments without 64 bit ints'); | ||
|
|
||
| /// A BitField over an enum (or other class whose values implement "index"). |
There was a problem hiding this comment.
nit: first sentence should be its own paragraph.
| /// See <https://www.dartlang.org/articles/numeric-computation/#smis-and-mints> | ||
| const int kMaxUnsignedSMI = 0x3FFFFFFFFFFFFFFF; | ||
|
|
||
| /// A BitField over an enum (or other class whose values implement "index"). |
There was a problem hiding this comment.
nit: first sentence should be its own paragraph.
| /// the bits are filled with zeros. | ||
| /// | ||
| /// The given length must be at most 62. | ||
| BitField.filled(int length, bool _) // ignore: avoid_unused_constructor_parameters |
There was a problem hiding this comment.
please add explanation to all // ignore: , see https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#comment-all--ignores
| /// | ||
| /// The given length must be at most 62. | ||
| BitField(int _length) | ||
| : assert(_length <= 62); |
There was a problem hiding this comment.
Should the 62 be a const here and below like in the other class?
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // BITFIELD |
There was a problem hiding this comment.
This heading comment seems kind of redundant now that it has its own file?
| /// Unsupported. | ||
| int get kMaxUnsignedSMI => throw UnsupportedError('Not supported in environments without 64 bit ints'); | ||
|
|
||
| /// A BitField over an enum (or other class whose values implement "index"). |
There was a problem hiding this comment.
Should the doc comments here and/or in the other implementation mention the conditions for when this thing is actually supported?
(I am also wondering how dartdocs deals with conditional imports? Does it only show the dart docs for one of the impls? If so, which one? Does this thing show up twice in dart docs now?)
There was a problem hiding this comment.
I don't think they should show up at all, but I'll need to test it.
Either the conditionally imported file has the same API, or we're in a world of hurt. That would mean all documentation should be on the main (non-conditional) implementation. I'm not sure if that means we should leave stub comments or something on the conditional file.
|
Turns out we don't need to do this quite yet |
allows compiling flutter in environments without 64 bit ints.