Cleanup handling of NullType in type propagation.

runtime/vm/intermediate_language.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/intermediate_language.h"
 
 #include "vm/bit_vector.h"
 #include "vm/dart_entry.h"
 #include "vm/flow_graph_allocator.h"
 #include "vm/flow_graph_builder.h"
@@ skipping 131 matching lines @@
 }
 
 
 void ForwardInstructionIterator::RemoveCurrentFromGraph() {
   current_ = current_->RemoveFromGraph(true);  // Set current_ to previous.
 }
 
 
 // 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;
   }
 
   // Functions that do not explicitly return a value, implicitly return null,
   // except generative constructors, which return the object being constructed.
   // It is therefore acceptable for void functions to return null.
   // In case of a null constant, we have already returned true above, else we
   // return false here.
   if (dst_type.IsVoidType()) {
     return false;
@@ skipping 438 matching lines @@
   return Type::DynamicType();
 }
 
 
 RawAbstractType* PolymorphicInstanceCallComp::CompileType() const {
   return Type::DynamicType();
 }
 
 
 RawAbstractType* StaticCallComp::CompileType() const {
-  return function().result_type();
+  if (FLAG_enable_type_checks) {
+    return function().result_type();
+  }
+  return Type::DynamicType();
 }
 
 
 RawAbstractType* LoadLocalComp::CompileType() const {
   if (FLAG_enable_type_checks) {
     return local().type().raw();
   }
   return Type::DynamicType();
 }
 
@@ skipping 413 matching lines @@
   BindInstr* right = right_use->definition()->AsBind();
   if (right == NULL) return NULL;
   ConstantVal* right_constant = right->computation()->AsConstant();
   if (right_constant == NULL) return NULL;
   // TODO(fschneider): Handle other cases: e === false and e !== true/false.
   // Handles e === true.
   if ((kind() == Token::kEQ_STRICT) &&
       (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.
     return left;
   }
   return NULL;
 }
 
 
 LocationSummary* StrictCompareComp::MakeLocationSummary() const {
@@ skipping 238 matching lines @@
   if (compiler->is_ssa()) {
     ASSERT(locs()->in(0).IsRegister());
     __ PushRegister(locs()->in(0).reg());
   }
 }
 
 
 #undef __
 
 }  // namespace dart