[wasm] Replace WasmName by Vector<const char>

[wasm] Replace WasmName by Vector<const char>

R=titzer@chromium.org

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

[titzer, 2016-04-22 13:35:20]

https://codereview.chromium.org/1910213004/diff/1/src/wasm/wasm-opcodes.h
File src/wasm/wasm-opcodes.h (left):

https://codereview.chromium.org/1910213004/diff/1/src/wasm/wasm-opcodes.h#old...
src/wasm/wasm-opcodes.h:52: struct WasmName {
You could as well introduce a typedef to keep the intent clear (and reduce the
amount of code changed).

[titzer, 2016-04-26 13:01:43]

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:196: DCHECK_EQ(static_cast<int64_t>(length),
static_cast<int>(length));
Why is there an int64 in this DCHECK? Seems a little suspicious. Did you mean
uint32_t?

[Clemens Hammacher, 2016-04-26 13:12:21]

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:196: DCHECK_EQ(static_cast<int64_t>(length),
static_cast<int>(length));
On 2016/04/26 at 13:01:43, titzer wrote:
> Why is there an int64 in this DCHECK? Seems a little suspicious. Did you mean
uint32_t?

The check ensures that there is no int-overflow.
You can't really compare an uint32_t to a int32_t (a.k.a. int), so I compare the
zero-extended length with the sign-extended length. The lefthand side is
zero-extended to int64, the righthand side is sign-extended. Both should be
equal.

[titzer, 2016-04-26 13:16:12]

Otherwise lgtm

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:196: DCHECK_EQ(static_cast<int64_t>(length),
static_cast<int>(length));
On 2016/04/26 13:12:21, Clemens Hammacher wrote:
> On 2016/04/26 at 13:01:43, titzer wrote:
> > Why is there an int64 in this DCHECK? Seems a little suspicious. Did you
mean
> uint32_t?
> 
> The check ensures that there is no int-overflow.
> You can't really compare an uint32_t to a int32_t (a.k.a. int), so I compare
the
> zero-extended length with the sign-extended length. The lefthand side is
> zero-extended to int64, the righthand side is sign-extended. Both should be
> equal.

Might as well just DCHECK_GE(length, 0), as we assume that sizeof(int) >=
sizeof(uint32_t)

[Clemens Hammacher, 2016-04-26 13:20:09]

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h
File src/wasm/wasm-module.h (right):

https://codereview.chromium.org/1910213004/diff/20001/src/wasm/wasm-module.h#...
src/wasm/wasm-module.h:196: DCHECK_EQ(static_cast<int64_t>(length),
static_cast<int>(length));
On 2016/04/26 at 13:16:11, titzer wrote:
> On 2016/04/26 13:12:21, Clemens Hammacher wrote:
> > On 2016/04/26 at 13:01:43, titzer wrote:
> > > Why is there an int64 in this DCHECK? Seems a little suspicious. Did you
mean
> > uint32_t?
> > 
> > The check ensures that there is no int-overflow.
> > You can't really compare an uint32_t to a int32_t (a.k.a. int), so I compare
the
> > zero-extended length with the sign-extended length. The lefthand side is
> > zero-extended to int64, the righthand side is sign-extended. Both should be
> > equal.
> 
> Might as well just DCHECK_GE(length, 0), as we assume that sizeof(int) >=
sizeof(uint32_t)

Yep, that's nicer. Changed it accordingly.

[Clemens Hammacher, 2016-04-26 14:43:00]

The CQ bit was checked by clemensh@chromium.org

[Clemens Hammacher, 2016-04-26 14:43:01]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/1910213004/#ps40001
(title: "nicer bounds check")

[commit-bot: I haz the power, 2016-04-26 14:43:09]

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

[commit-bot: I haz the power, 2016-04-26 14:47:20]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-26 14:47:39]

Description was changed from

==========
[wasm] Replace WasmName by Vector<const char>

R=titzer@chromium.org
==========

to

==========
[wasm] Replace WasmName by Vector<const char>

R=titzer@chromium.org

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

[commit-bot: I haz the power, 2016-04-26 14:47:41]

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