Ticket #1540 (closed deprecation: fixed)

Opened 5 years ago

Last modified 4 years ago

Current COW strings to be replaced with immutable version.

Reported by: bacek Owned by: bacek
Priority: normal Milestone: 2.5
Component: none Version: trunk
Severity: medium Keywords:
Cc: Language:
Patch status: Platform:

Description

This will improve performance, reduce codebase, etc.

Various "inplace" string modification functions to be removed.

Attachments

tt1540_imcc.patch Download (1.0 KB) - added by plobsing 4 years ago.
inplace concat IMCC patch

Change History

  Changed 5 years ago by bacek

Also all Parrot_str_foo functions that have AGRMOD(STRING *dest) output parameter will be changed.

  Changed 5 years ago by bacek

Also 2-args inplace forms of string ops are deprecated. E.g. chopn, etc.

  Changed 5 years ago by coke

See r45730 : -1 from me.

I would recommend that instead of picking a new name, that we just refer to this as substr, but with a different signature.

  Changed 5 years ago by bacek

Coke, why?

"substr" produce substring of original string. "replace" produce string with replaced part.

-- Bacek

  Changed 5 years ago by bacek

We do need separate replace_s_s_i_i_c op because current substr_s_s_i_i_c has different (and bad) semantic.

-- Bacek

  Changed 5 years ago by coke

bacek - Ok, +0 from me. =-)

  Changed 5 years ago by coke

  • owner set to bacek

Bacek - can we get an explicit list of what is left to do on this ticket?

  Changed 5 years ago by coke

  • owner changed from bacek to coke

Only inout string opcode left seems to be concat_s_s. (pin/unpin are not covered by this ticket.)

  Changed 5 years ago by coke

Note that just removing this opcode is insufficient. ".=" in PIR desugars to this, so we need to fix this to use a suitable remaining opcode.

  Changed 5 years ago by coke

This ticket also covers the inplace 'reverse' method on String. An acceptable fix is to make this an instance method that operates on the String itself instead of a passed in string.

  Changed 5 years ago by julian.notfound@…

>  This ticket also covers the inplace 'reverse' method on String. An
>  acceptable fix is to make this an instance method that operates on the
>  String itself instead of a passed in string.

The 'trans' method also modifies in place its argument.


-- 
Salu2

follow-ups: ↓ 14 ↓ 15   Changed 4 years ago by coke

  • owner changed from coke to plobsing

assign to plobsing for remaining IMCC work.

  Changed 4 years ago by gerd

  • milestone changed from 2.4 to 2.5

in reply to: ↑ 12   Changed 4 years ago by plobsing

Replying to coke:

assign to plobsing for remaining IMCC work.

What does changing opcodes which mutate registers, which are *pointers* to strings, not the strings themselves, have to do with COW vs immutable strings? This seems like a red herring to me.

Changed 4 years ago by plobsing

inplace concat IMCC patch

in reply to: ↑ 12 ; follow-up: ↓ 16   Changed 4 years ago by plobsing

  • owner changed from plobsing to bacek

Replying to coke:

assign to plobsing for remaining IMCC work.

I've attached a patch for the IMCC work. Still not really sure this is worthwhile.

in reply to: ↑ 15   Changed 4 years ago by bacek

Replying to plobsing:

I've attached a patch for the IMCC work. Still not really sure this is worthwhile.

+1 for applying patch.

-- Bacek

  Changed 4 years ago by bacek

Hello.

Actually String PMC methods are _not_ covered (and shouldn't be). Ticket was about STRING*. So, after applying plobsing's patch and removing concat_s_s opcode ticket can be closed.

-- Bacek

  Changed 4 years ago by Vasily Chekalkin

  • status changed from new to closed
  • resolution set to fixed

Remove concat_s_s op and rebootstrap generated files. Closes #1540

Changeset: ddbdd541c6cf9d1d9deb2a4d589bf1fcae416b12

Note: See TracTickets for help on using tickets.