Add Dart_IsFuture. To be used in Dartium implementation of Promises.

Add Dart_IsFuture. To be used in Dartium implementation of Promises.

BUG=
R=asiva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=40577

[rmacnak, 2014-09-19 17:12:30]

rmacnak@google.com changed reviewers:
+ asiva@google.com

[siva, 2014-09-19 17:38:36]

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1878: if (obj.IsInstance()) {
ReusableObjectHandleScope reused_obj_handle(isolate);
const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
if (!obj.IsNull() {
  ...
  ...
}
return false;

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1879: const Library& core_lib =
Library::Handle(Library::AsyncLibrary());
replace core_lib with async_lib for the variable name.

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1881:
Class::Handle(core_lib.LookupClass(Symbols::Future()));
Class::Handle(isolate, ....);

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1885: bool is_future =
obj_class.IsSubtypeOf(TypeArguments::Handle(isolate),
IsSubTypeOf(Object::null_type_arguments(),
            future_class,
            Object::null_type_arguments(),
            &malformed_type_error);

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl_tes...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl_tes...
runtime/vm/dart_api_impl_test.cc:1401: EXPECT(Dart_IsFuture(result));
You probably also want a negative test:

Dart_Handle zero = Dart_NewInteger(0);
EXPECT(!Dart_IsFuture(zero));
Dart_Handle str = NewString("I am not a Future");
EXPECT(!Dart_IsFuture(str));

[rmacnak, 2014-09-19 21:32:03]

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1878: if (obj.IsInstance()) {
On 2014/09/19 17:38:35, siva wrote:
> ReusableObjectHandleScope reused_obj_handle(isolate);
> const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
> if (!obj.IsNull() {
>   ...
>   ...
> }
> return false;

Trips the assert as the reusable handle is used in LookupClass.

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1879: const Library& core_lib =
Library::Handle(Library::AsyncLibrary());
On 2014/09/19 17:38:35, siva wrote:
> replace core_lib with async_lib for the variable name.

Done.

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1881:
Class::Handle(core_lib.LookupClass(Symbols::Future()));
On 2014/09/19 17:38:35, siva wrote:
> Class::Handle(isolate, ....);

Done.

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1885: bool is_future =
obj_class.IsSubtypeOf(TypeArguments::Handle(isolate),
On 2014/09/19 17:38:36, siva wrote:
> IsSubTypeOf(Object::null_type_arguments(),
>             future_class,
>             Object::null_type_arguments(),
>             &malformed_type_error);

Done. (And two other occurrences in this file.)

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl_tes...
File runtime/vm/dart_api_impl_test.cc (right):

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl_tes...
runtime/vm/dart_api_impl_test.cc:1401: EXPECT(Dart_IsFuture(result));
On 2014/09/19 17:38:36, siva wrote:
> You probably also want a negative test:
> 
> Dart_Handle zero = Dart_NewInteger(0);
> EXPECT(!Dart_IsFuture(zero));
> Dart_Handle str = NewString("I am not a Future");
> EXPECT(!Dart_IsFuture(str));

Added

[siva, 2014-09-19 21:59:38]

lgtm

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc
File runtime/vm/dart_api_impl.cc (right):

https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
runtime/vm/dart_api_impl.cc:1878: if (obj.IsInstance()) {
On 2014/09/19 21:32:02, rmacnak wrote:
> On 2014/09/19 17:38:35, siva wrote:
> > ReusableObjectHandleScope reused_obj_handle(isolate);
> > const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
> > if (!obj.IsNull() {
> >   ...
> >   ...
> > }
> > return false;
> 
> Trips the assert as the reusable handle is used in LookupClass.

In that case you could restructure it as:

Class obj_classs = Class::Handle(isolate);
{
  ReusableObjectHandleScope reused_obj_handle(isolate);
  const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
  if (obj.IsNull() {
    return false;
  }
  obj_class ^= obj.clazz();
}
....
....
....
return is_future;

[rmacnak, 2014-09-22 23:05:22]

On 2014/09/19 21:59:38, siva wrote:
> lgtm
> 
> https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc
> File runtime/vm/dart_api_impl.cc (right):
> 
>
https://codereview.chromium.org/585643004/diff/1/runtime/vm/dart_api_impl.cc#...
> runtime/vm/dart_api_impl.cc:1878: if (obj.IsInstance()) {
> On 2014/09/19 21:32:02, rmacnak wrote:
> > On 2014/09/19 17:38:35, siva wrote:
> > > ReusableObjectHandleScope reused_obj_handle(isolate);
> > > const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
> > > if (!obj.IsNull() {
> > >   ...
> > >   ...
> > > }
> > > return false;
> > 
> > Trips the assert as the reusable handle is used in LookupClass.
> 
> In that case you could restructure it as:
> 
> Class obj_classs = Class::Handle(isolate);
> {
>   ReusableObjectHandleScope reused_obj_handle(isolate);
>   const Instance& obj = Api::UnwrapInstanceHandle(reused_obj, handle);
>   if (obj.IsNull() {
>     return false;
>   }
>   obj_class ^= obj.clazz();
> }
> ....
> ....
> ....
> return is_future;

Seems a bit much to avoid one handle allocation in a function that isn't on a
hot path. Leaving as is.

[rmacnak, 2014-09-22 23:06:03]

Committed patchset #2 (id:20001) manually as 40577 (presubmit successful).