n-api: use Maybe version of SetPrototype

[danbev]

Currently the following two warnings are displayed when compiling:

../src/node_api.cc:1966:12: warning: 'SetPrototype' is deprecated
[-Wdeprecated-declarations]
  wrapper->SetPrototype(proto);
           ^

../src/node_api.cc:1967:8: warning: 'SetPrototype' is deprecated
[-Wdeprecated-declarations]
  obj->SetPrototype(wrapper);

This commit changes these calls to use the Maybe version.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

n-api

[danbev]

CI: https://ci.nodejs.org/job/node-test-pull-request/8526/

[TimothyGu]

LGTM, assuming it's established that SetPrototype cannot ever return an empty value.

[gibfahn]

cc/ @nodejs/n-api

[bnoordhuis]

This looks wrong to me; not your changes, but the logic itself. What happens when proto->IsNull() is true? What happens when obj is a proxy object?

[jasongin]

What happens when proto->IsNull() is true?

That should be fine. If it was correct for the original object to have a null prototype, then it is correct for the object inserted in the prototype chain to have a null prototype.

What happens when obj is a proxy object?

Is it not valid to set the prototype of a proxy object? If V8 doesn't allow it, then the SetPrototype() call will presumably return an empty Maybe, and this code should check for that and return an error status in that case.

[danbev]

@jasongin Thanks for the details. @bnoordhuis Does this seems reasonable to you too?

[danbev]

@bnoordhuis If you get a chance could you take a look and see if you think I can land this, thanks!

[bnoordhuis]

The point about proxy objects still stand. Since they can intercept (and fail) almost any operation, you have to be prepared for exceptions.

[danbev]

Ah I see. Let me take a look and come up with a suggestion. Thanks

[bnoordhuis]

@danbev Any luck? That warning irritates me more by the day. Happy to take over if you don't have time.

[danbev]

@bnoordhuis Sorry, I've not had time yet so please take over if you do have time. Thanks

[bnoordhuis]

#14053 landed, I'll close this.