[wasm] Remove the module environment and signature as arguments to OpcodeArity.

[wasm] Remove the module environment and signature as arguments to OpcodeArity.

R=rossberg@chromium.org,ahaas@chromium.org
BUG=

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

[rossberg, 2016-05-02 14:42:13]

lgtm

[ahaas, 2016-05-02 14:48:29]

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:290: return operand.arity;
Could the arity be calculated in a static function?

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1525: WasmDecoder decoder(nullptr, nullptr, pc, end);
Could you make OpcodeArity a static function?

[titzer, 2016-05-02 14:57:31]

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:290: return operand.arity;
On 2016/05/02 14:48:28, ahaas wrote:
> Could the arity be calculated in a static function?

Not sure what value that would add, since this function already handles all
bytecodes. Or do you mean to suggest that this functionality should be instead
moved to the top-level functions OpcodeArity and OpcodeLength?

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1525: WasmDecoder decoder(nullptr, nullptr, pc, end);
On 2016/05/02 14:48:28, ahaas wrote:
> Could you make OpcodeArity a static function?

The decoder classes are not exposed through the header file, but instead a
top-level function, so that would require adding a wrapper class.

[ahaas, 2016-05-02 15:12:41]

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc
File src/wasm/ast-decoder.cc (right):

https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
src/wasm/ast-decoder.cc:1525: WasmDecoder decoder(nullptr, nullptr, pc, end);
On 2016/05/02 at 14:57:31, titzer wrote:
> On 2016/05/02 14:48:28, ahaas wrote:
> > Could you make OpcodeArity a static function?
> 
> The decoder classes are not exposed through the header file, but instead a
top-level function, so that would require adding a wrapper class.

What I mean is, I don't see a point why you have to make a WasmDecoder object to
call decoder.OpcodeArity. Could also just be a personal preference.

[titzer, 2016-05-02 15:20:30]

On 2016/05/02 15:12:41, ahaas wrote:
> https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc
> File src/wasm/ast-decoder.cc (right):
> 
>
https://codereview.chromium.org/1937083002/diff/1/src/wasm/ast-decoder.cc#new...
> src/wasm/ast-decoder.cc:1525: WasmDecoder decoder(nullptr, nullptr, pc, end);
> On 2016/05/02 at 14:57:31, titzer wrote:
> > On 2016/05/02 14:48:28, ahaas wrote:
> > > Could you make OpcodeArity a static function?
> > 
> > The decoder classes are not exposed through the header file, but instead a
> top-level function, so that would require adding a wrapper class.
> 
> What I mean is, I don't see a point why you have to make a WasmDecoder object
to
> call decoder.OpcodeArity. Could also just be a personal preference.

This function used to use to the module environment and signature. Now that it
doesn't, the body could move into the main top-level function. However, the
similar function OpcodeLength() is used in the loop assignment analysis, and
moving its body into the top-level function that takes (pc, end) would introduce
a minor inefficiency in constructing another Decoder instance every time. Given
this is all in the .cc file, I think it's Ok this way.

[ahaas, 2016-05-02 15:21:00]

lgtm

[titzer, 2016-05-02 15:21:16]

The CQ bit was checked by titzer@chromium.org

[commit-bot: I haz the power, 2016-05-02 15:21:20]

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

[commit-bot: I haz the power, 2016-05-02 15:23:19]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-02 15:24:59]

Description was changed from

==========
[wasm] Remove the module environment and signature as arguments to OpcodeArity.

R=rossberg@chromium.org,ahaas@chromium.org
BUG=
==========

to

==========
[wasm] Remove the module environment and signature as arguments to OpcodeArity.

R=rossberg@chromium.org,ahaas@chromium.org
BUG=

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

[commit-bot: I haz the power, 2016-05-02 15:25:00]

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