Issue 10389063: Implement PrimitiveOrNull types.

Issue 10389063: Implement PrimitiveOrNull types. (Closed)

Created:
8 years, 7 months ago by ngeoffray

Modified:
8 years, 7 months ago

Reviewers:
Lasse Reichstein Nielsen, floitsch

CC:
reviews_dartlang.org, karlklose, ahe, kasperl, Lasse Reichstein Nielsen

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Implement PrimitiveOrNull types. Committed: https://code.google.com/p/dart/source/detail?r=7537

Patch Set 3 : #

Created: 8 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+633 lines, -53 lines)			Patch
M	frog/tests/leg/type_combination_test.dart	View	1	31 chunks	+367 lines, -2 lines	0 comments	Download
M	lib/compiler/implementation/dart2js.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	lib/compiler/implementation/ssa/builder.dart	View	1	1 chunk	+0 lines, -5 lines	0 comments	Download
M	lib/compiler/implementation/ssa/optimize.dart	View	1	2 chunks	+6 lines, -4 lines	0 comments	Download
M	lib/compiler/implementation/ssa/types.dart	View	1	14 chunks	+241 lines, -39 lines	0 comments	Download
M	tests/language/language.status	View	1	1 chunk	+4 lines, -2 lines	0 comments	Download
M	tests/lib/lib.status	View	1 2	1 chunk	+8 lines, -0 lines	0 comments	Download
M	tests/standalone/standalone.status	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download
M	tests/utils/utils.status	View	1	1 chunk	+3 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Lasse Reichstein Nielsen

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implementation/ssa/types.dart File lib/compiler/implementation/ssa/types.dart (right): https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implementation/ssa/types.dart#newcode54 lib/compiler/implementation/ssa/types.dart:54: static final HType BOOLEAN_OR_NULL = const HBooleanOrNullType(); DO you ...

8 years, 7 months ago (2012-05-10 10:15:20 UTC) #2

floitsch

LGTM with comments. Please update leg/type_test. https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implementation/ssa/types.dart File lib/compiler/implementation/ssa/types.dart (right): https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implementation/ssa/types.dart#newcode10 lib/compiler/implementation/ssa/types.dart:10: * supertype, or ...

8 years, 7 months ago (2012-05-10 10:29:43 UTC) #3

ngeoffray

8 years, 7 months ago (2012-05-11 10:17:54 UTC) #4

Thanks Florian and Lasse for having taken a look.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
File lib/compiler/implementation/ssa/types.dart (right):

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:10: * supertype, or the type [type].
On 2012/05/10 10:29:43, floitsch wrote:
> Returns an [HType] that represents [type] and all types that have [type] as
> supertype.

Done.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:17: // TODO(ngeoffray): Replace
object type type [type].
On 2012/05/10 10:29:43, floitsch wrote:
> type type [type] ?

Done.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:54: static final HType
BOOLEAN_OR_NULL = const HBooleanOrNullType();
On 2012/05/10 10:15:20, Lasse Reichstein Nielsen wrote:
> DO you think it will eventually be worth it to have general sum types?

Yes. As discussed, I preferred special casing these types because we already
special case their non-null equivalent. And lots of our optimizations assume
something like isInteger to not be null.

But for general types, I believe we will have a more generalized abstraction for
sum types.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:217: if (other.isNumberOrNull())
return HType.NUMBER_OR_NULL;
On 2012/05/10 10:29:43, floitsch wrote:
> not correct for integers or doubles.

Done.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:406: return HType.CONFLICTING;
On 2012/05/10 10:29:43, floitsch wrote:
> Add comment why indexable + null-string is conflicting.

Done.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:576: if
(other.isIndexablePrimitive()) return HType.READABLE_ARRAY;
On 2012/05/10 10:29:43, floitsch wrote:
> no. If other isString and combine is used for intersection, then we must
return
> HType.CONFLICTING.
> We can't use combine for both union and intersection here.

Done.

https://chromiumcodereview.appspot.com/10389063/diff/2001/lib/compiler/implem...
lib/compiler/implementation/ssa/types.dart:587: if
(other.isIndexablePrimitive()) return HType.STRING;
On 2012/05/10 10:29:43, floitsch wrote:
> ditto.

Done.

Expand Messages | Collapse Messages