https://codereview.chromium.org/1201783003/diff/1/src/scanner.cc
File src/scanner.cc (right):
https://codereview.chromium.org/1201783003/diff/1/src/scanner.cc#newcode1436
src/scanner.cc:1436: bool Scanner::ContainsDot() {
On 2015/06/22 07:53:43, Sven Panne wrote:
Instead of using caveman-style loops, the right way to implement this
is to make
Scanner/LiteralBuffer obey the iterator protocol. With that,
everything is more
flexible and ContainsDot would be a one-liner with std::find, so it's
probably
not even worth having as a separate function.
Turns out Vector<> provides begin/end, which allow some greater brevity.
LiteralBuffer manages providing subranges (in multiple byte sizes),
which it returns as Vector<>.
I suppose it could provide these via two separate methods, but these in
turn would have to be carried through two layers of wrapper out to
Scanner. (The convience methods in Scanner are presumably due to the
number of uses, i.e. many).
Having this as a method allows the DCHECK (which judging by the
underlying implementation seems to be allowed to be null under some
circumstances?)
It might be worth incorporating into DoubleValue(), but see below.
https://codereview.chromium.org/1201783003/diff/1/src/scanner.cc#newcode1438
src/scanner.cc:1438: Vector<const uint8_t> literal_string =
literal_one_byte_string();
On 2015/06/22 07:08:16, titzer wrote:
Does this make a copy of the bytes underneath?
No, it copies a reference to the underying buffer and size (And is a
weak reference in contrast to ScopedVector).
When I'd asked about leaning on the compiler though, what I'd meant was
how frugal about copies of Vector<>? (The other functions here,
DoubleValue for instance seem to assume instantiating a Vector<>
(costing 2 pointers, times 2 layers of return), is an acceptable cost,
presumably hoping for optimization.)
DoubleValue() is only used twice, once needing to know about dots, the
other not. So I could add a bool* with_dot parameter to it, but that
struck me as odd.
Trying to calibrate my micro-optimization instincts for this code base.
Sorry coming from NaCl core, which is filled with C code (for instance,
we're literally having a should we allow any C++ in corner X of the
codebase discussion next week :-).
https://codereview.chromium.org/1201783003/diff/1/src/scanner.cc#newcode1441
src/scanner.cc:1441: with_dot |= (literal_string[i] == '.');
On 2015/06/22 07:08:16, titzer wrote:
Early return?
Replaced.
FWIW was lifted wholesale from a prototype CL Andreas pointed me at :-)
https://codereview.chromium.org/1201783003/diff/20001/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
https://codereview.chromium.org/1201783003/diff/20001/test/cctest/test-parsing.cc#newcode1105
test/cctest/test-parsing.cc:1105: v8::V8::Initialize();
Any pointers on how to reduce this?
Speaking of refactoring, I'm tempted to roll this boilerplate setup into
something common, shared at least with the one above? Though I've heard
testing nerds suggest hard to setup objects are code smell of sorts.
https://codereview.chromium.org/1201783003/
--
--
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.
