Thanks, like it, though I'm not so sure about the naming...
https://codereview.chromium.org/335293004/diff/80001/src/ast-value-factory.h
File src/ast-value-factory.h (right):
https://codereview.chromium.org/335293004/diff/80001/src/ast-value-factory.h#newcode281
src/ast-value-factory.h:281: const AstString* GetOneByteString(const
Vector<const uint8_t>& literal);
On 2014/06/23 11:46:45, marja wrote:
On 2014/06/23 09:44:27, rossberg wrote:
> Hm, isn't the idea of the Vector abstraction that this shouldn't be
necessary?
It's not a huge difference; this will copy the length and the data
pointer (not
the actual data); I just got a comment in another code review that I
should use
const Vector&; and why not... that's copying one pointer instead of 2.
But you pay with an extra indirection for all uses. Consequently, this
is not only a micro optimisation, it's one that could actually cost
micro performance. ;)
https://codereview.chromium.org/335293004/diff/130035/src/ast-value-factory.h
File src/ast-value-factory.h (right):
https://codereview.chromium.org/335293004/diff/130035/src/ast-value-factory.h#newcode60
src/ast-value-factory.h:60: // Creates a cons string. The only allowed
operations for cons strings are
On 2014/06/23 11:46:45, marja wrote:
On 2014/06/23 09:44:27, rossberg wrote:
> Disallowing a whole set of operations in some cases is kind of
nasty, as is
> always using space for the cross product of both possible
representations.
>
> Do you think it would be possible to have the two different forms of
strings
as
> concrete subclasses of (an abstract) AstString (say AstConsString
and
> AstRawString?). Then you can get rid of the type thingy and a bunch
of
> associated asserts, potentially use virtual dispatch for the common
functions,
> and checked downcasts to get to the representation-specific ones.
Done.
I named the classes AstStringBase, AstString (the same as AstString
used to be
before) and AstConsString.
Most places use AstStrings (like before), only function names are
stored as
AstStringBase.
Hm, from a stylistic point, I don't like naming an abstract interface
something-Base -- that's usually reserved for pure implementation
inheritance, not interface inheritance.
Would renaming AstString->AstRawString, AstStringBase->AstString be
particularly terrible?
We don't need to downcast ever. The only needed operations for
cons strings are string() and length(), and AstStringBase takes care
of them.
Ah, even better then.
https://codereview.chromium.org/335293004/diff/230001/src/ast-value-factory.cc
File src/ast-value-factory.cc (right):
https://codereview.chromium.org/335293004/diff/230001/src/ast-value-factory.cc#newcode129
src/ast-value-factory.cc:129: // AstStrings are internalized before
AstConsStrings so left and right are
Maybe ASSERT that here?
https://codereview.chromium.org/335293004/
--
--
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.
