Cleanup handling of NullType in type propagation.

runtime/vm/intermediate_language.cc

Index: runtime/vm/intermediate_language.cc
===================================================================
--- runtime/vm/intermediate_language.cc	(revision 10752)
+++ runtime/vm/intermediate_language.cc	(working copy)
@@ -149,28 +149,39 @@
 
 // Default implementation of visiting basic blocks.  Can be overridden.
 void FlowGraphVisitor::VisitBlocks() {
+  ASSERT(current_iterator_ == NULL);
   for (intptr_t i = 0; i < block_order_.length(); ++i) {
     BlockEntryInstr* entry = block_order_[i];
     entry->Accept(this);
-    for (ForwardInstructionIterator it(entry); !it.Done(); it.Advance()) {
+    ForwardInstructionIterator it(entry);
+    current_iterator_ = &it;
+    for (; !it.Done(); it.Advance()) {
       it.Current()->Accept(this);
     }
+    current_iterator_ = NULL;
   }
 }
 
 
+// Returns true if the value is constant null.
+bool Value::IsConstantNull() const {
+  return IsConstant() && AsConstant()->value().IsNull();
+}

[srdjan, 2012/08/15 17:49:56]

How about making it virtual and implementing in UseVal as return false and in
ConstantVal as return value.IsNull() ?

[regis, 2012/08/15 22:49:18]

Good point. It is a bit more complex than just returning false from a UseVal,
though, because a UseVal may point to a BindInstr with a ConstantVal as
computation. See my code. 

+
+
 // Returns true if the compile type of this value is more specific than the
 // given dst_type.
 // TODO(regis): Support a set of compile types for the given value.
 bool Value::CompileTypeIsMoreSpecificThan(const AbstractType& dst_type) const {
-  ASSERT(!dst_type.IsMalformed());  // Should be tested by caller.
-  ASSERT(!dst_type.IsDynamicType());  // Should be tested by caller.
-  ASSERT(!dst_type.IsObjectType());  // Should be tested by caller.
+  // No type is more specific than a malformed type.
+  if (dst_type.IsMalformed()) {
+    return false;
+  }
 
   // If the value is the null constant, its type (NullType) is more specific
   // than the destination type, even if the destination type is the void type,
   // since a void function is allowed to return null.
-  if (IsConstant() && AsConstant()->value().IsNull()) {
+  if (IsConstantNull()) {
     return true;
   }
 
@@ -629,7 +640,10 @@
 
 
 RawAbstractType* StaticCallComp::CompileType() const {
-  return function().result_type();
+  if (FLAG_enable_type_checks) {
+    return function().result_type();
+  }
+  return Type::DynamicType();
 }
 
 
@@ -1063,6 +1077,11 @@
       (right_constant->value().raw() == Bool::True()) &&
       left_use->CompileTypeIsMoreSpecificThan(
           Type::Handle(Type::BoolInterface()))) {
+    // TODO(regis): I am not sure this optimization is correct.
+    // Although the compile type of left_use is bool, left_use may be null at
+    // runtime, e.g. true === f(), with bool f() { bool x; return x }.
+    // In this case, the optimized strict equal returns null.

[srdjan, 2012/08/15 17:49:56]

See also TODO(srdjan) above

[regis, 2012/08/15 22:49:18]

I removed my TODO, now that the problem is understood.

+
     // Remove the constant from the graph.
     right->RemoveFromGraph();
     // Return left subexpression as the replacement for this instruction.