I've become more enamored with the immutable strings idea as I read
through the branch diff (immutable_strings_part1). It's quite impressive
how much redundant code gets removed when we can rely on a string to
never change.
The branch does a good job of hiding the details from the user too. That
is, for the most part HLL code can run ahead as usual, and never need to
know that a given string operation has changed a register/variable to
point to a different string header.
A few comments:
* Since opcodes like 'downcase' are changing to return a value rather
than modifying one, then the opcodes should be clear about it. That is,
deprecate the one argument:
downcase $S0
In favor of the two argument:
$S1 = downcase $S0
* We've lost the semantic difference between 'set' and 'assign' for
strings, they're now identical. This isn't necessarily a bad thing, but
it seems sensible to deprecate 'assign' for strings and keep it only for
PMCs where it's meaningful.
* In src/string/encoding/fixed_8.c there's a stray FIXME in the wrong
comment style (// instead of /* */) which I'm guessing someone intends
to remove before merging.
* In src/string/api.c, does 'string_capacity' really make sense anymore
as a function? Shouldn't the capacity of an immutable string always be
the same as the length of the string? Possibly another deprecation item.
* Can Parrot_str_append be changed to just call Parrot_str_concat? Or,
perhaps deprecate append, since they notionally do the same thing now.
* It sure removes a lot of cruft from substr to have it only return a
new value.
* Another stray wrong-style comment in src/string/api.c, in
Parrot_str_pin on a call to Parrot_str_write_COW. Again on another call
to the same function later in the same file.
* A bit of complexity added to join, but unavoidable with immutable
strings. At least it avoids generating a pile of temporary strings by
allocating enough memory to hold the whole result at once.
Three cheers for unconventional thinking, finding speed gains and code
simplifications in unexpected places.
Allison
_______________________________________________
http://lists.parrot.org/mailman/listinfo/parrot-dev
