First round of comments.
General note: Please use either parameter_count or param_count consistently.
https://codereview.chromium.org/530783002/diff/1/src/compiler/linkage.h
File src/compiler/linkage.h (right):
https://codereview.chromium.org/530783002/diff/1/src/compiler/linkage.h#newcode202
src/compiler/linkage.h:202: MachineType GetParameterType(int index) {
Nit: const
https://codereview.chromium.org/530783002/diff/1/src/compiler/linkage.h#newcode212
src/compiler/linkage.h:212: MachineType GetReturnType() { return
incoming_->GetReturnType(0); }
Nit: const
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h
File src/compiler/machine-type.h (right):
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode116
src/compiler/machine-type.h:116: Signature(uint8_t return_count,
uint16_t param_count, T* reps)
The Chrome Coding Style says: "Use unsigned integer types (preferably
size_t) for object and allocation sizes, object counts, array and
pointer offsets, vector indices, and so on. The signed types are
incorrect and unsafe for these purposes. (Integer overflow behavior for
signed types is undefined in the C and C++ standards, while the behavior
is defined for unsigned types.) The C++ STL is a guide here: they use
size_t and foo::size_type for very good reasons.", and esp. "Interfaces
(function prototypes) for these uses should use size_t, even if the
underlying storage is a smaller unsigned type (e.g. as a space
optimization)."
So please use size_t for the interface, and DCHECK the range if you use
uintX_t for internal representation.
See http://www.chromium.org/developers/coding-style
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode119
src/compiler/machine-type.h:119: int ReturnCount() const { return
return_count_; }
size_t return_count() const { return return_count_; }
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode120
src/compiler/machine-type.h:120: int ParamCount() const { return
param_count_; }
size_t param_count() const { return param_count_; }
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode122
src/compiler/machine-type.h:122: T GetParam(int index) const {
size_t for index
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode123
src/compiler/machine-type.h:123: DCHECK(index >= 0 && index <
param_count_);
DCHECK_LT(index, param_count());
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode124
src/compiler/machine-type.h:124: return reps_[return_count_ + index];
reps_[return_count() + index]
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode127
src/compiler/machine-type.h:127: T GetReturn(int index = 0) const {
size_t for index
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode128
src/compiler/machine-type.h:128: DCHECK(index >= 0 && index <
return_count_);
DCHECK_LT(index, return_count());
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode143
src/compiler/machine-type.h:143: const int return_count_;
size_t
https://codereview.chromium.org/530783002/diff/1/src/compiler/machine-type.h#newcode144
src/compiler/machine-type.h:144: const int param_count_;
size_t
https://codereview.chromium.org/530783002/diff/1/src/compiler/raw-machine-assembler.h
File src/compiler/raw-machine-assembler.h (right):
https://codereview.chromium.org/530783002/diff/1/src/compiler/raw-machine-assembler.h#newcode57
src/compiler/raw-machine-assembler.h:57: int parameter_count() { return
machine_sig_->ParamCount(); }
Nit: size_t + const
https://codereview.chromium.org/530783002/diff/1/src/compiler/structured-machine-assembler.h
File src/compiler/structured-machine-assembler.h (right):
https://codereview.chromium.org/530783002/diff/1/src/compiler/structured-machine-assembler.h#newcode73
src/compiler/structured-machine-assembler.h:73: int parameter_count() {
return machine_sig_->ParamCount(); }
Nit: size_t + const
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h
File test/cctest/compiler/call-tester.h (right):
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode144
test/cctest/compiler/call-tester.h:144: int param_count = 5;
Nit: size_t
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode146
test/cctest/compiler/call-tester.h:146: for (int i = param_count - 1; i
= 0; i--) {
for (; param_count != 0 && types[param_count - 1] == kMachNone;
--param_count)
;
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode149
test/cctest/compiler/call-tester.h:149: int return_count = return_type
== kMachNone ? 0 : 1;
size_t
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode154
test/cctest/compiler/call-tester.h:154: for (int i = 0; i < param_count;
i++) {
size_t
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode162
test/cctest/compiler/call-tester.h:162: void VerifyParameters(int
parameter_count, MachineType* parameter_types) {
size_t
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/call-tester.h#newcode164
test/cctest/compiler/call-tester.h:164: for (int i = 0; i <
parameter_count; i++) {
size_t
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/graph-builder-tester.h
File test/cctest/compiler/graph-builder-tester.h (right):
https://codereview.chromium.org/530783002/diff/1/test/cctest/compiler/graph-builder-tester.h#newcode51
test/cctest/compiler/graph-builder-tester.h:51: int parameter_count()
const { return machine_sig_->ParamCount(); }
size_t
https://codereview.chromium.org/530783002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
