Reviewers: Michael Starzinger,

Description:
Reenable 17167: "Ensure lower <= upper bound"

Fixed handlification bug (see 2nd patch).

Will handlify Type::Union and Type::Intersect in separate CL.

[email protected]
BUG=

Please review this at https://codereview.chromium.org/27164003/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+69, -19 lines):
  M src/types.h
  M src/types.cc
  M src/typing.cc
  M test/cctest/test-types.cc


Index: src/types.cc
diff --git a/src/types.cc b/src/types.cc
index 9716686f25f1a4a285a8b6b4cf6bfd7f277a7128..17a19b29e4263ac4ef2907c0c9ba45d62baa4cf5 100644
--- a/src/types.cc
+++ b/src/types.cc
@@ -240,6 +240,7 @@ int Type::GlbBitset() {
 // Check this <= that.
 bool Type::SlowIs(Type* that) {
   // Fast path for bitsets.
+  if (this->is_none()) return true;
   if (that->is_bitset()) {
     return (this->LubBitset() | that->as_bitset()) == that->as_bitset();
   }
@@ -526,9 +527,13 @@ void Type::TypePrint(FILE* out) {
     }
     PrintF(out, "}");
   } else if (is_constant()) {
-    PrintF(out, "Constant(%p)", static_cast<void*>(*as_constant()));
+    PrintF(out, "Constant(%p : ", static_cast<void*>(*as_constant()));
+    from_bitset(LubBitset())->TypePrint(out);
+    PrintF(")");
   } else if (is_class()) {
-    PrintF(out, "Class(%p)", static_cast<void*>(*as_class()));
+    PrintF(out, "Class(%p < ", static_cast<void*>(*as_class()));
+    from_bitset(LubBitset())->TypePrint(out);
+    PrintF(")");
   } else if (is_union()) {
     PrintF(out, "{");
     Handle<Unioned> unioned = as_union();
Index: src/types.h
diff --git a/src/types.h b/src/types.h
index 80772d841af3b06c057bab9d13d976f9d634e12c..5d437e26b243654691102507ca6a6986f2e46fad 100644
--- a/src/types.h
+++ b/src/types.h
@@ -226,6 +226,7 @@ class Type : public Object {
     kUnusedEOL = 0
   };

+  bool is_none() { return this == None(); }
   bool is_bitset() { return this->IsSmi(); }
   bool is_class() { return this->IsMap(); }
   bool is_constant() { return this->IsBox(); }
@@ -299,10 +300,18 @@ struct Bounds {
   Handle<Type> upper;

   Bounds() {}
-  Bounds(Handle<Type> l, Handle<Type> u) : lower(l), upper(u) {}
-  Bounds(Type* l, Type* u, Isolate* isl) : lower(l, isl), upper(u, isl) {}
-  explicit Bounds(Handle<Type> t) : lower(t), upper(t) {}
-  Bounds(Type* t, Isolate* isl) : lower(t, isl), upper(t, isl) {}
+  Bounds(Handle<Type> l, Handle<Type> u) : lower(l), upper(u) {
+    ASSERT(lower->Is(upper));
+  }
+  Bounds(Type* l, Type* u, Isolate* isl) : lower(l, isl), upper(u, isl) {
+    ASSERT(lower->Is(upper));
+  }
+  explicit Bounds(Handle<Type> t) : lower(t), upper(t) {
+    ASSERT(lower->Is(upper));
+  }
+  Bounds(Type* t, Isolate* isl) : lower(t, isl), upper(t, isl) {
+    ASSERT(lower->Is(upper));
+  }

   // Unrestricted bounds.
   static Bounds Unbounded(Isolate* isl) {
@@ -311,9 +320,11 @@ struct Bounds {

   // Meet: both b1 and b2 are known to hold.
   static Bounds Both(Bounds b1, Bounds b2, Isolate* isl) {
-    return Bounds(
-        handle(Type::Union(b1.lower, b2.lower), isl),
-        handle(Type::Intersect(b1.upper, b2.upper), isl));
+    Handle<Type> lower(Type::Union(b1.lower, b2.lower), isl);
+    Handle<Type> upper(Type::Intersect(b1.upper, b2.upper), isl);
+    // Lower bounds are considered approximate, correct as necessary.
+    lower = handle(Type::Intersect(lower, upper), isl);
+    return Bounds(lower, upper);
   }

   // Join: either b1 or b2 is known to hold.
@@ -324,10 +335,14 @@ struct Bounds {
   }

   static Bounds NarrowLower(Bounds b, Handle<Type> t, Isolate* isl) {
+    // Lower bounds are considered approximate, correct as necessary.
+    t = handle(Type::Intersect(t, b.upper), isl);
     return Bounds(handle(Type::Union(b.lower, t), isl), b.upper);
   }
   static Bounds NarrowUpper(Bounds b, Handle<Type> t, Isolate* isl) {
-    return Bounds(b.lower, handle(Type::Intersect(b.upper, t), isl));
+    return Bounds(
+        handle(Type::Intersect(b.lower, t), isl),
+        handle(Type::Intersect(b.upper, t), isl));
   }
 };

Index: src/typing.cc
diff --git a/src/typing.cc b/src/typing.cc
index 65a20cf38e89a845543159a7a4984ff471191a7e..b30a7b39d00ed838597dae2dcd0226a1ea99ef7a 100644
--- a/src/typing.cc
+++ b/src/typing.cc
@@ -252,8 +252,8 @@ void AstTyper::VisitForStatement(ForStatement* stmt) {
     RECURSE(Visit(stmt->cond()));
   }
   RECURSE(Visit(stmt->body()));
-  store_.Forget();  // Control may transfer here via 'continue'.
   if (stmt->next() != NULL) {
+    store_.Forget();  // Control may transfer here via 'continue'.
     RECURSE(Visit(stmt->next()));
   }
store_.Forget(); // Control may transfer here via termination or 'break'. @@ -585,7 +585,9 @@ void AstTyper::VisitBinaryOperation(BinaryOperation* expr) {
       Type* upper = Type::Union(
           expr->left()->bounds().upper, expr->right()->bounds().upper);
       if (!upper->Is(Type::Signed32())) upper = Type::Signed32();
-      NarrowType(expr, Bounds(Type::Smi(), upper, isolate_));
+      Type* lower = Type::Intersect(
+          handle(Type::Smi(), isolate_), handle(upper, isolate_));
+      NarrowType(expr, Bounds(lower, upper, isolate_));
       break;
     }
     case Token::BIT_XOR:
@@ -598,7 +600,8 @@ void AstTyper::VisitBinaryOperation(BinaryOperation* expr) {
     case Token::SHR:
       RECURSE(Visit(expr->left()));
       RECURSE(Visit(expr->right()));
-      NarrowType(expr, Bounds(Type::Smi(), Type::Unsigned32(), isolate_));
+      // TODO(rossberg): we could use an UnsignedSmi as lower bound here...
+      NarrowType(expr, Bounds(Type::Unsigned32(), isolate_));
       break;
     case Token::ADD: {
       RECURSE(Visit(expr->left()));
@@ -606,15 +609,17 @@ void AstTyper::VisitBinaryOperation(BinaryOperation* expr) {
       Bounds l = expr->left()->bounds();
       Bounds r = expr->right()->bounds();
       Type* lower =
-          l.lower->Is(Type::Number()) && r.lower->Is(Type::Number()) ?
-              Type::Smi() :
+          l.lower->Is(Type::None()) || r.lower->Is(Type::None()) ?
+              Type::None() :
           l.lower->Is(Type::String()) || r.lower->Is(Type::String()) ?
-              Type::String() : Type::None();
+              Type::String() :
+          l.lower->Is(Type::Number()) && r.lower->Is(Type::Number()) ?
+              Type::Smi() : Type::None();
       Type* upper =
-          l.upper->Is(Type::Number()) && r.upper->Is(Type::Number()) ?
-              Type::Number() :
           l.upper->Is(Type::String()) || r.upper->Is(Type::String()) ?
-              Type::String() : Type::NumberOrString();
+              Type::String() :
+          l.upper->Is(Type::Number()) && r.upper->Is(Type::Number()) ?
+              Type::Number() : Type::NumberOrString();
       NarrowType(expr, Bounds(lower, upper, isolate_));
       break;
     }
Index: test/cctest/test-types.cc
diff --git a/test/cctest/test-types.cc b/test/cctest/test-types.cc
index 823af0b1abf97fcb7debf134de95a9c5b1138181..264d2ed881b3781c0d31e809b2b25475248c4edd 100644
--- a/test/cctest/test-types.cc
+++ b/test/cctest/test-types.cc
@@ -318,6 +318,11 @@ TEST(Is) {
   CheckUnordered(T.Array, T.Function);

   // Structured subtyping
+  CheckSub(T.None, T.ObjectClass);
+  CheckSub(T.None, T.ObjectConstant1);
+  CheckSub(T.ObjectClass, T.Any);
+  CheckSub(T.ObjectConstant1, T.Any);
+
   CheckSub(T.ObjectClass, T.Object);
   CheckSub(T.ArrayClass, T.Object);
   CheckUnordered(T.ObjectClass, T.ArrayClass);
@@ -384,6 +389,9 @@ TEST(Maybe) {
   CheckDisjoint(T.Object, T.Proxy);
   CheckDisjoint(T.Array, T.Function);

+  CheckOverlap(T.ObjectClass, T.Any);
+  CheckOverlap(T.ObjectConstant1, T.Any);
+
   CheckOverlap(T.ObjectClass, T.Object);
   CheckOverlap(T.ArrayClass, T.Object);
   CheckOverlap(T.ObjectClass, T.ObjectClass);
@@ -432,6 +440,8 @@ TEST(Union) {
   CHECK(IsUnion(Type::Union(T.ObjectClass, T.ArrayClass)));

   CheckEqual(T.Union(T.ObjectClass, T.ObjectClass), T.ObjectClass);
+  CheckSub(T.None, T.Union(T.ObjectClass, T.ArrayClass));
+  CheckSub(T.Union(T.ObjectClass, T.ArrayClass), T.Any);
   CheckSub(T.ObjectClass, T.Union(T.ObjectClass, T.ArrayClass));
   CheckSub(T.ArrayClass, T.Union(T.ObjectClass, T.ArrayClass));
   CheckSub(T.Union(T.ObjectClass, T.ArrayClass), T.Object);
@@ -447,6 +457,8 @@ TEST(Union) {
CheckEqual(T.Union(T.ObjectConstant1, T.ObjectConstant1), T.ObjectConstant1); CheckEqual(T.Union(T.ArrayConstant1, T.ArrayConstant1), T.ArrayConstant1); CheckEqual(T.Union(T.ArrayConstant1, T.ArrayConstant1), T.ArrayConstant2);
+  CheckSub(T.None, T.Union(T.ObjectConstant1, T.ObjectConstant2));
+  CheckSub(T.Union(T.ObjectConstant1, T.ObjectConstant2), T.Any);
CheckSub(T.ObjectConstant1, T.Union(T.ObjectConstant1, T.ObjectConstant2)); CheckSub(T.ObjectConstant2, T.Union(T.ObjectConstant1, T.ObjectConstant2));
   CheckSub(T.ArrayConstant2, T.Union(T.ArrayConstant1, T.ObjectConstant2));
@@ -463,6 +475,7 @@ TEST(Union) {
   CHECK(IsUnion(Type::Union(T.ObjectClass, T.Number)));

   CheckEqual(T.Union(T.ObjectClass, T.Object), T.Object);
+  CheckSub(T.None, T.Union(T.ObjectClass, T.Number));
   CheckSub(T.Union(T.ObjectClass, T.Number), T.Any);
   CheckSub(T.Union(T.ObjectClass, T.Smi), T.Union(T.Object, T.Number));
   CheckSub(T.Union(T.ObjectClass, T.Array), T.Object);
@@ -477,6 +490,7 @@ TEST(Union) {

   CheckEqual(T.Union(T.SmiConstant, T.Number), T.Number);
   CheckEqual(T.Union(T.ObjectConstant1, T.Object), T.Object);
+  CheckSub(T.None, T.Union(T.ObjectConstant1, T.Number));
   CheckSub(T.Union(T.ObjectConstant1, T.Number), T.Any);
CheckSub(T.Union(T.ObjectConstant1, T.Signed32), T.Union(T.Object, T.Number));
   CheckSub(T.Union(T.ObjectConstant1, T.Array), T.Object);
@@ -489,6 +503,8 @@ TEST(Union) {
   CHECK(IsUnion(Type::Union(T.ObjectConstant1, T.ObjectClass)));
   CHECK(IsUnion(Type::Union(T.ArrayClass, T.ObjectConstant2)));

+  CheckSub(T.None, T.Union(T.ObjectConstant1, T.ArrayClass));
+  CheckSub(T.Union(T.ObjectConstant1, T.ArrayClass), T.Any);
   CheckSub(T.Union(T.ObjectConstant1, T.ArrayClass), T.Object);
   CheckSub(T.ObjectConstant1, T.Union(T.ObjectConstant1, T.ArrayClass));
   CheckSub(T.ArrayClass, T.Union(T.ObjectConstant1, T.ArrayClass));
@@ -518,6 +534,9 @@ TEST(Union) {
       T.ObjectConstant1,
       T.Union(T.Union(T.ArrayClass, T.ObjectConstant1), T.Double));
   CheckSub(
+      T.None,
+      T.Union(T.Union(T.ArrayClass, T.ObjectConstant1), T.Double));
+  CheckSub(
       T.Union(T.Union(T.ArrayClass, T.ObjectConstant1), T.Double),
       T.Any);
   CheckSub(
@@ -534,6 +553,12 @@ TEST(Union) {
       T.Union(T.ObjectClass, T.Union(T.ObjectConstant1, T.ObjectClass)),
       T.Union(T.ObjectClass, T.ObjectConstant1));
   CheckSub(
+      T.None,
+      T.Union(T.ObjectClass, T.Union(T.ObjectConstant1, T.ObjectClass)));
+  CheckSub(
+      T.Union(T.ObjectClass, T.Union(T.ObjectConstant1, T.ObjectClass)),
+      T.Any);
+  CheckSub(
       T.Union(T.ObjectClass, T.Union(T.ObjectConstant1, T.ObjectClass)),
       T.Object);
   CheckEqual(


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to