Skip to content

Commit

Permalink
switch to using check function for most repeated fields
Browse files Browse the repository at this point in the history
For extensions, messages, groups, and enums, the
generated code calls PbList directly and needs to
be migrated.

For the other types, create a PbList without specifying
a generic type and rely only on the check function.

BUG=
[email protected]

Review URL: https://chromiumcodereview.appspot.com//1283093003.
  • Loading branch information
Brian Slesinsky committed Aug 13, 2015
1 parent 9660948 commit 5e603ae
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 129 deletions.
48 changes: 3 additions & 45 deletions lib/src/protobuf/builder_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,52 +47,10 @@ class BuilderInfo {
makeDefault, subBuilder, null);
}

// Repeated non-message.
// Repeated, not a message, group, or enum.
void p(int tagNumber, String name, int fieldType) {
MakeDefaultFunc makeDefault;
switch (fieldType & ~0x7) {
case FieldType._BOOL_BIT:
makeDefault = () => new PbList<bool>();
break;
case FieldType._BYTES_BIT:
makeDefault = () => new PbList<List<int>>();
break;
case FieldType._STRING_BIT:
makeDefault = () => new PbList<String>();
break;
case FieldType._FLOAT_BIT:
makeDefault = () => PbList.createFloat();
break;
case FieldType._DOUBLE_BIT:
makeDefault = () => new PbList<double>();
break;
case FieldType._ENUM_BIT:
makeDefault = () => new PbList<ProtobufEnum>();
break;
case FieldType._INT32_BIT:
case FieldType._SINT32_BIT:
case FieldType._SFIXED32_BIT:
makeDefault = () => PbList.createSigned32();
break;
case FieldType._UINT32_BIT:
case FieldType._FIXED32_BIT:
makeDefault = () => PbList.createUnsigned32();
break;
case FieldType._INT64_BIT:
case FieldType._SINT64_BIT:
case FieldType._SFIXED64_BIT:
makeDefault = () => PbList.createSigned64();
break;
case FieldType._UINT64_BIT:
case FieldType._FIXED64_BIT:
makeDefault = () => PbList.createUnsigned64();
break;
case FieldType._MESSAGE_BIT:
throw new ArgumentError('use BuilderInfo.m() for repeated messages');
default:
throw new ArgumentError('unknown type ${fieldType}');
}

// The fieldType entirely determines the check function.
var makeDefault = () => new PbList.forFieldType(fieldType);
add(tagNumber, name, fieldType, makeDefault, null, null);
}

Expand Down
165 changes: 109 additions & 56 deletions lib/src/protobuf/field_error.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ part of protobuf;
/// Returns the error message for an invalid field value,
/// or null if it's valid.
///
/// For enums and message fields, this check is only approximate,
/// For enums, group, and message fields, this check is only approximate,
/// because the exact type isn't included in [fieldType].
String _getFieldError(int fieldType, var value) {
switch (FieldType._baseType(fieldType)) {
Expand All @@ -30,46 +30,31 @@ String _getFieldError(int fieldType, var value) {
case FieldType._ENUM_BIT:
if (value is! ProtobufEnum) return 'not type ProtobufEnum';
return null;

case FieldType._INT32_BIT:
if (value is! int) return 'not type int';
if (!_isSigned32(value)) return 'out of range for int32';
return null;
case FieldType._INT64_BIT:
if (value is! Int64) return 'not Int64';
if (!_isSigned64(value)) return 'out of range for int64';
return null;
case FieldType._SINT32_BIT:
case FieldType._SFIXED32_BIT:
if (value is! int) return 'not type int';
if (!_isSigned32(value)) return 'out of range for sint32';
return null;
case FieldType._SINT64_BIT:
if (value is! Int64) return 'not Int64';
if (!_isSigned64(value)) return 'out of range for sint64';
if (!_isSigned32(value)) return 'out of range for signed 32-bit int';
return null;

case FieldType._UINT32_BIT:
if (value is! int) return 'not type int';
if (!_isUnsigned32(value)) return 'out of range for uint32';
return null;
case FieldType._UINT64_BIT:
if (value is! Int64) return 'not Int64';
if (!_isUnsigned64(value)) return 'out of range for uint64';
return null;
case FieldType._FIXED32_BIT:
if (value is! int) return 'not type int';
if (!_isUnsigned32(value)) return 'out of range for fixed32';
if (!_isUnsigned32(value)) return 'out of range for unsigned 32-bit int';
return null;

case FieldType._INT64_BIT:
case FieldType._SINT64_BIT:
case FieldType._UINT64_BIT:
case FieldType._FIXED64_BIT:
if (value is! Int64) return 'not Int64';
if (!_isUnsigned64(value)) return 'out of range for fixed64';
return null;
case FieldType._SFIXED32_BIT:
if (value is! int) return 'not type int';
if (!_isSigned32(value)) return 'out of range for sfixed32';
return null;
case FieldType._SFIXED64_BIT:
// We always use the full range of the same Dart type.
// It's up to the caller to treat the Int64 as signed or unsigned.
// See: https://github.com/dart-lang/dart-protobuf/issues/44
if (value is! Int64) return 'not Int64';
if (!_isSigned64(value)) return 'out of range for sfixed64';
return null;

case FieldType._GROUP_BIT:
case FieldType._MESSAGE_BIT:
if (value is! GeneratedMessage) return 'not a GeneratedMessage';
Expand All @@ -79,52 +64,120 @@ String _getFieldError(int fieldType, var value) {
}
}

// Checking functions for repeated fields.
/// Returns a function for validating items in a repeated field.
///
/// For enum, group, and message fields, the check is only approximate,
/// because the exact type isn't included in [fieldType].
CheckFunc getCheckFunction(int fieldType) {
switch (fieldType & ~0x7) {
case FieldType._BOOL_BIT:
return _checkBool;
case FieldType._BYTES_BIT:
return _checkBytes;
case FieldType._STRING_BIT:
return _checkString;
case FieldType._FLOAT_BIT:
return _checkFloat;
case FieldType._DOUBLE_BIT:
return _checkDouble;

case FieldType._INT32_BIT:
case FieldType._SINT32_BIT:
case FieldType._SFIXED32_BIT:
return _checkSigned32;

case FieldType._UINT32_BIT:
case FieldType._FIXED32_BIT:
return _checkUnsigned32;

case FieldType._INT64_BIT:
case FieldType._SINT64_BIT:
case FieldType._SFIXED64_BIT:
case FieldType._UINT64_BIT:
case FieldType._FIXED64_BIT:
// We always use the full range of the same Dart type.
// It's up to the caller to treat the Int64 as signed or unsigned.
// See: https://github.com/dart-lang/dart-protobuf/issues/44
return _checkAnyInt64;

case FieldType._ENUM_BIT:
return _checkAnyEnum;
case FieldType._GROUP_BIT:
case FieldType._MESSAGE_BIT:
return _checkAnyMessage;
}
throw new ArgumentError('check function not implemented: ${fieldType}');
}

// check functions for repeated fields

void _checkNotNull(val) {
if (val == null) {
throw new ArgumentError("Can't add a null to a repeated field");
}
}

void _checkBool(bool val) {
if (val is! bool) throw _createFieldTypeError(val, 'a bool');
}

void _checkNothing(x) {}
void _checkBytes(List<int> val) {
if (val is! List<int>) throw _createFieldTypeError(val, 'a List<int>');
}

void _checkString(String val) {
if (val is! String) throw _createFieldTypeError(val, 'a String');
}

void _checkFloat(double val) {
if (!_isFloat32(val)) {
throw new ArgumentError(
'Illegal to add value (${val}): out of range for float');
}
_checkDouble(val);
if (!_isFloat32(val)) throw _createFieldRangeError(val, 'a float');
}

void _checkDouble(double val) {
if (val is! double) throw _createFieldTypeError(val, 'a double');
}

void _checkInt(int val) {
if (val is! int) throw _createFieldTypeError(val, 'an int');
}

void _checkSigned32(int val) {
if (!_isSigned32(val)) {
throw new ArgumentError(
'Illegal to add value (${val}): out of range for int32');
}
_checkInt(val);
if (!_isSigned32(val)) throw _createFieldRangeError(val, 'a signed int32');
}

void _checkUnsigned32(int val) {
_checkInt(val);
if (!_isUnsigned32(val)) {
throw new ArgumentError(
'Illegal to add value (${val}): out of range for uint32');
throw _createFieldRangeError(val, 'an unsigned int32');
}
}

void _checkSigned64(Int64 val) {
if (!_isSigned64(val)) {
throw new ArgumentError(
'Illegal to add value (${val}): out of range for sint64');
}
void _checkAnyInt64(Int64 val) {
if (val is! Int64) throw _createFieldTypeError(val, 'an Int64');
}

void _checkUnsigned64(Int64 val) {
if (!_isUnsigned64(val)) {
throw new ArgumentError(
'Illegal to add value (${val}): out of range for uint64');
_checkAnyEnum(ProtobufEnum val) {
if (val is! ProtobufEnum) throw _createFieldTypeError(val, 'a ProtobufEnum');
}

_checkAnyMessage(GeneratedMessage val) {
if (val is! GeneratedMessage) {
throw _createFieldTypeError(val, 'a GeneratedMessage');
}
// It is up to the caller to treat the Int64 as unsigned
// even though we're using the signed type. (The full range is used.)
}

ArgumentError _createFieldTypeError(val, String wantedType) =>
new ArgumentError('Value ($val) is not ${wantedType}');

RangeError _createFieldRangeError(val, String wantedType) =>
new RangeError('Value ($val) is not ${wantedType}');

bool _inRange(min, value, max) => (min <= value) && (value <= max);

bool _isSigned32(int value) => _inRange(-2147483648, value, 2147483647);
bool _isUnsigned32(int value) => _inRange(0, value, 4294967295);
bool _isSigned64(Int64 value) => _isUnsigned64(value);
bool _isUnsigned64(Int64 value) => value is Int64;
bool _isFloat32(double value) => value.isNaN || value.isInfinite ||
_inRange(-3.4028234663852886E38, value, 3.4028234663852886E38);
bool _isFloat32(double value) => value.isNaN ||
value.isInfinite ||
_inRange(-3.4028234663852886E38, value, 3.4028234663852886E38);
2 changes: 1 addition & 1 deletion lib/src/protobuf/field_type.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class FieldType {

// Closures commonly used by initializers.
static String _STRING_EMPTY() => '';
static List<int> _BYTES_EMPTY() => new PbList<int>();
static List<int> _BYTES_EMPTY() => new PbList(check: _checkInt);
static bool _BOOL_FALSE() => false;
static int _INT_ZERO() => 0;
static double _DOUBLE_ZERO() => 0.0;
Expand Down
32 changes: 10 additions & 22 deletions lib/src/protobuf/pb_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,19 @@ part of protobuf;
typedef CheckFunc(x);

class PbList<E> extends Object with ListMixin<E> implements List<E> {

final List<E> _wrappedList;
final CheckFunc check;

PbList({this.check: _checkNothing}) : _wrappedList = <E>[];
PbList({this.check: _checkNotNull}) : _wrappedList = <E>[];

PbList.from(List from)
: _wrappedList = new List<E>.from(from),
check = _checkNothing;

static PbList<int> createSigned32() =>
new PbList<int>(check: _checkSigned32);
static PbList<int> createUnsigned32() =>
new PbList<int>(check: _checkUnsigned32);
static PbList<Int64> createSigned64() =>
new PbList<Int64>(check: _checkSigned64);
static PbList<Int64> createUnsigned64() =>
new PbList<Int64>(check: _checkUnsigned64);
static PbList<double> createFloat() =>
new PbList<double>(check: _checkFloat);

bool operator ==(other) =>
(other is PbList) && _areListsEqual(other, this);
: _wrappedList = new List<E>.from(from),
check = _checkNotNull;

factory PbList.forFieldType(int fieldType) =>
new PbList(check: getCheckFunction(fieldType));

bool operator ==(other) => (other is PbList) && _areListsEqual(other, this);

int get hashCode {
int hash = 0;
Expand Down Expand Up @@ -148,12 +138,10 @@ class PbList<E> extends Object with ListMixin<E> implements List<E> {
int get length => _wrappedList.length;

void _validate(E val) {
if (val == null) {
throw new ArgumentError('Value is null');
}
check(val);
// TODO: remove after migration to check functions is finished
if (val is! E) {
throw new ArgumentError('Value ($val) is not of the correct type');
}
check(val);
}
}
10 changes: 5 additions & 5 deletions test/list_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void main() {
});

test('PbList for signed int32 validates items', () {
var list = PbList.createSigned32();
var list = new PbList.forFieldType(FieldType.P3);

expect(() {
list.add(-2147483649);
Expand All @@ -119,7 +119,7 @@ void main() {
});

test('PBList for unsigned int32 validates items', () {
var list = PbList.createUnsigned32();
var list = new PbList.forFieldType(FieldType.PU3);

expect(() {
list.add(-1);
Expand All @@ -139,7 +139,7 @@ void main() {
});

test('PbList for float validates items', () {
var list = PbList.createFloat();
var list = new PbList.forFieldType(FieldType.PF);

expect(() {
list.add(3.4028234663852886E39);
Expand All @@ -159,7 +159,7 @@ void main() {
});

test('PbList for signed Int64 validates items', () {
PbList<Int64> list = PbList.createSigned64();
var list = new PbList.forFieldType(FieldType.P6);
expect(() {
list.add(cast(0)); // not an Int64
}, badArgument);
Expand All @@ -178,7 +178,7 @@ void main() {
});

test('PbList for unsigned Int64 validates items', () {
PbList<Int64> list = PbList.createUnsigned64();
var list = new PbList.forFieldType(FieldType.PU6);
expect(() {
list.add(cast(0)); // not an Int64
}, badArgument);
Expand Down

0 comments on commit 5e603ae

Please sign in to comment.