Fox issue 2108: throw illegal argument exception for RegExp natives. Continue throwin NPE where exp…

Fox issue 2108: throw illegal argument exception for RegExp natives. Continue throwin NPE where expected.
Committed: https://code.google.com/p/dart/source/detail?r=5343

[siva, 2012-03-12 18:49:00]

LGTM

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc
File runtime/lib/regexp.cc (right):

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc#n...
runtime/lib/regexp.cc:26: CheckAndThrowExceptionIfNull(arg1);
Not sure how many times this comes up where we have to check for Null and proper
type. If there are too many of these we could have a GET_NONNULL_NATIVE_ARGUMENT
macro.

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc#n...
runtime/lib/regexp.cc:27: GET_NATIVE_ARGUMENT(String, pattern,
arguments->At(1));
Another unrelated question, the macro GET_NATIVE_ARGUMENT
calls CheckedHandle(arg);
if arg is not an instance it results in a FATAL(...).

Should we consider throwing an exception there instead of a FATAL?

[srdjan, 2012-03-12 19:01:13]

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc
File runtime/lib/regexp.cc (right):

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc#n...
runtime/lib/regexp.cc:26: CheckAndThrowExceptionIfNull(arg1);
On 2012/03/12 18:49:01, asiva wrote:
> Not sure how many times this comes up where we have to check for Null and
proper
> type. If there are too many of these we could have a
GET_NONNULL_NATIVE_ARGUMENT
> macro.

JSSyntaxRegExp are the only place where an illegal argument may throw a NPE.
Maybe we remove this special behavior at some point.

https://chromiumcodereview.appspot.com/9669054/diff/1/runtime/lib/regexp.cc#n...
runtime/lib/regexp.cc:27: GET_NATIVE_ARGUMENT(String, pattern,
arguments->At(1));
On 2012/03/12 18:49:01, asiva wrote:
> Another unrelated question, the macro GET_NATIVE_ARGUMENT
> calls CheckedHandle(arg);
> if arg is not an instance it results in a FATAL(...).
> 
> Should we consider throwing an exception there instead of a FATAL?

Generated code can never pass a non-Instance object. We could make it quicker by
eliminating CheckedHandle and check that the argument is instance in debug mode
only.