Replace calls to Class::IsParameterized() by either

Replace calls to Class::IsParameterized() by either
  Class::NumTypeParameters() > 0
or by
  Class::NumTypeArguments() > 0.
Committed: https://code.google.com/p/dart/source/detail?r=503

[regis, 2011-10-18 00:59:34]

TBR

[siva, 2011-10-18 05:06:19]

http://codereview.chromium.org/8329005/diff/1/runtime/vm/ast.h
File runtime/vm/ast.h (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/ast.h#newcode1670
runtime/vm/ast.h:1670: ASSERT(type_arguments_.IsNull() ||
Should the dropped assert be replaced by
ASSERT((class::Handle(constructor_.owner()).NumTypeArguments() == 0) ||

http://codereview.chromium.org/8329005/diff/1/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/code_generator.cc#ne...
runtime/vm/code_generator.cc:153: if (num_type_arguments > 0) {
The comment and code in the if statement is not matching the condition, should
be:
if (num_type_arguments == 0) {
   .....
}

http://codereview.chromium.org/8329005/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/object.h#newcode428
runtime/vm/object.h:428: }
The old comment says IsParametrized is more efficient than calling
NumTypeArguments(), if that is valid then will we be regressing?

Also I am wondering if it would make the code more
readable if you continued to have IsParameterized and
implemented it as:
bool IsParameterized() const {
  return NumTypeArguments() > 0;
}

[regis, 2011-10-18 17:28:21]

Thanks Siva!

I will address your comments and fix the bug you found in a following
changelist.

-- Regis

http://codereview.chromium.org/8329005/diff/1/runtime/vm/ast.h
File runtime/vm/ast.h (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/ast.h#newcode1670
runtime/vm/ast.h:1670: ASSERT(type_arguments_.IsNull() ||
On 2011/10/18 05:06:19, asiva wrote:
> Should the dropped assert be replaced by
> ASSERT((class::Handle(constructor_.owner()).NumTypeArguments() == 0) ||

Actually, I realized that we should not be passing type arguments if the class
does not accept any. So removing the first condition is tightening the assert
the right way.

http://codereview.chromium.org/8329005/diff/1/runtime/vm/code_generator.cc
File runtime/vm/code_generator.cc (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/code_generator.cc#ne...
runtime/vm/code_generator.cc:153: if (num_type_arguments > 0) {
On 2011/10/18 05:06:19, asiva wrote:
> The comment and code in the if statement is not matching the condition, should
> be:
> if (num_type_arguments == 0) {
>    .....
> }

Good catch. I think we avoided this new bug, because the inlined allocation did
the job every time.

http://codereview.chromium.org/8329005/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

http://codereview.chromium.org/8329005/diff/1/runtime/vm/object.h#newcode428
runtime/vm/object.h:428: }
On 2011/10/18 05:06:19, asiva wrote:
> The old comment says IsParametrized is more efficient than calling
> NumTypeArguments(), if that is valid then will we be regressing?
> 
> Also I am wondering if it would make the code more
> readable if you continued to have IsParameterized and
> implemented it as:
> bool IsParameterized() const {
>   return NumTypeArguments() > 0;
> }

Yes, I think I should reintroduce a helper function for efficiency and
readability. But I need to pick a better name, since this one was not clear.