Make handle ctors explicit

Make handle ctors explicit

Without this change, the v8::Local<> constructor will be picked up by the
compiler as an option for an implicit cast for any pointer type.  This leads
to bad error messages when accidentally passing an erroneous pointer type to
a function wanting a Local<> (complains about a pointer assignment in Local<>'s
constructor as opposed to a bad type for the parameter of the function being
called) and also causes ambiguity errors where none should exist when calling
overloaded functions (for example a function taking either a std::string or a
v8::Local<v8::Script> cannot be called with a const char * because the compiler
sees both types as being constructable with a const char *).

R=jochen@chromium.org
BUG=

Committed: https://crrev.com/b6c9b70356ca4d2d36424a384290baaa7ba85207
Cr-Commit-Position: refs/heads/master@{#33602}

[jochen (gone - plz use gerrit), 2016-01-28 09:19:20]

The CQ bit was checked by jochen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-28 09:19:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1647833005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1647833005/1

[commit-bot: I haz the power, 2016-01-28 09:52:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-28 09:52:07]

Dry run: This issue passed the CQ dry run.

[jochen (gone - plz use gerrit), 2016-01-28 09:54:33]

I wonder whether just adding "explicit" to the constructor would have the same
effect

[jochen (gone - plz use gerrit), 2016-01-28 18:09:16]

I just saw that you replied by email. Could you use the "reply" method on the
web interface in the future, so the discussion is visible here?

to your point, I'm fine with dropping the template parameter, but I think we
should still add "explicit" to the ctor.

[xaxxon, 2016-01-29 01:20:36]

On 2016/01/28 18:09:16, jochen wrote:
> I just saw that you replied by email. Could you use the "reply" method on the
> web interface in the future, so the discussion is visible here?
> 
> to your point, I'm fine with dropping the template parameter, but I think we
> should still add "explicit" to the ctor.

Sorry about that, I didn't realize it didn't send the email to the system to be
added.   I added the "explicit" attribute.

[xaxxon, 2016-01-29 01:26:03]

On 2016/01/29 01:20:36, xaxxon wrote:
> On 2016/01/28 18:09:16, jochen wrote:
> > I just saw that you replied by email. Could you use the "reply" method on
the
> > web interface in the future, so the discussion is visible here?
> > 
> > to your point, I'm fine with dropping the template parameter, but I think we
> > should still add "explicit" to the ctor.
> 
> Sorry about that, I didn't realize it didn't send the email to the system to
be
> added.   I added the "explicit" attribute.

Glancing through v8.h, I noticed another place where this type of syntax is
being used where it doesn't do anything except add complexity and potential
weird errors:

  template <class S> V8_INLINE Persistent(S* that) : PersistentBase<T>(that) { }

where PersistentBase<T> takes its one parameter and assigns it to a T*.  

Any arguments against changing this in the same way?

[xaxxon, 2016-01-29 01:33:42]

On 2016/01/29 01:26:03, xaxxon wrote:
> On 2016/01/29 01:20:36, xaxxon wrote:
> > On 2016/01/28 18:09:16, jochen wrote:
> > > I just saw that you replied by email. Could you use the "reply" method on
> the
> > > web interface in the future, so the discussion is visible here?
> > > 
> > > to your point, I'm fine with dropping the template parameter, but I think
we
> > > should still add "explicit" to the ctor.
> > 
> > Sorry about that, I didn't realize it didn't send the email to the system to
> be
> > added.   I added the "explicit" attribute.
> 
> Glancing through v8.h, I noticed another place where this type of syntax is
> being used where it doesn't do anything except add complexity and potential
> weird errors:
> 
>   template <class S> V8_INLINE Persistent(S* that) : PersistentBase<T>(that) {
}
> 
> where PersistentBase<T> takes its one parameter and assigns it to a T*.  
> 
> Any arguments against changing this in the same way?

I submitted the additional change since our timezones are so far off we won't
have much back-and-forth time.  If you don't like it for some reason, I don't
care nearly as much about this one, as it isn't as externally visible - but,
like in the original spot, it seems like it can't be wrong.

[jochen (gone - plz use gerrit), 2016-01-29 08:32:14]

Description was changed from

==========
Without this change, the v8::Local<> constructor will be picked up by the
compiler as an option for an implicit cast
for any pointer type.  This leads to bad error messages when accidentally
passing an erroneous pointer type to
a function wanting a Local<> (complains about a pointer assignment in Local<>'s
constructor as opposed to a bad
type for the parameter of the function being called) and also causes ambiguity
errors where none should exist when
calling overloaded functions (for example a function taking either a std::string
or a v8::Local<v8::Script> cannot
be called with a const char * because the compiler sees both types as being
constructable with a const char *).

I believe this change limits the types the compiler believes match Local's
constructor parameter type to the types
that can actually be assigned to the internal data member Local::val_.

This is my first patch, so please let me know if there's anything else I need to
do.

Thank you.

--Zac
xaxxon@gmail.com

R=jochen@chromium.org
BUG=
==========

to

==========
Make handle ctors explicit

Without this change, the v8::Local<> constructor will be picked up by the
compiler as an option for an implicit cast for any pointer type.  This leads
to bad error messages when accidentally passing an erroneous pointer type to
a function wanting a Local<> (complains about a pointer assignment in Local<>'s
constructor as opposed to a bad type for the parameter of the function being
called) and also causes ambiguity errors where none should exist when calling
overloaded functions (for example a function taking either a std::string or a 
v8::Local<v8::Script> cannot be called with a const char * because the compiler
sees both types as being constructable with a const char *).

R=jochen@chromium.org
BUG=
==========

[jochen (gone - plz use gerrit), 2016-01-29 08:32:33]

thx, lgtm

I updated the CL description to match the required format

[jochen (gone - plz use gerrit), 2016-01-29 08:32:37]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2016-01-29 08:32:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1647833005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1647833005/40001

[commit-bot: I haz the power, 2016-01-29 09:12:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-29 09:13:02]

Description was changed from

==========
Make handle ctors explicit

Without this change, the v8::Local<> constructor will be picked up by the
compiler as an option for an implicit cast for any pointer type.  This leads
to bad error messages when accidentally passing an erroneous pointer type to
a function wanting a Local<> (complains about a pointer assignment in Local<>'s
constructor as opposed to a bad type for the parameter of the function being
called) and also causes ambiguity errors where none should exist when calling
overloaded functions (for example a function taking either a std::string or a 
v8::Local<v8::Script> cannot be called with a const char * because the compiler
sees both types as being constructable with a const char *).

R=jochen@chromium.org
BUG=
==========

to

==========
Make handle ctors explicit

Without this change, the v8::Local<> constructor will be picked up by the
compiler as an option for an implicit cast for any pointer type.  This leads
to bad error messages when accidentally passing an erroneous pointer type to
a function wanting a Local<> (complains about a pointer assignment in Local<>'s
constructor as opposed to a bad type for the parameter of the function being
called) and also causes ambiguity errors where none should exist when calling
overloaded functions (for example a function taking either a std::string or a
v8::Local<v8::Script> cannot be called with a const char * because the compiler
sees both types as being constructable with a const char *).

R=jochen@chromium.org
BUG=

Committed: https://crrev.com/b6c9b70356ca4d2d36424a384290baaa7ba85207
Cr-Commit-Position: refs/heads/master@{#33602}
==========

[commit-bot: I haz the power, 2016-01-29 09:13:03]

Patchset 3 (id:??) landed as
https://crrev.com/b6c9b70356ca4d2d36424a384290baaa7ba85207
Cr-Commit-Position: refs/heads/master@{#33602}