From: sam.saffron@...
Date: 2018-02-27T07:03:22+00:00
Subject: [ruby-core:85841] [Ruby trunk Feature#14478] String #uminus should de-dupe unconditionally

Issue #14478 has been updated by sam.saffron (Sam Saffron).


I just tried the trivial patch of:

```
diff --git a/string.c b/string.c
index ebf5618..a9b991f 100644
--- a/string.c
+++ b/string.c
@@ -2605,20 +2605,16 @@ str_uplus(VALUE str)
  * call-seq:
  *   -str  -> str (frozen)
  *
- * If the string is frozen, then return the string itself.
+ * Return a frozen, possibly pre-existing
+ * copy of the string.
  *
- * If the string is not frozen, return a frozen, possibly pre-existing
- * copy of it.
+ * String will be deduplicated as long as it is not tainted,
+ * or has any instance vars set on it.
  */
 static VALUE
 str_uminus(VALUE str)
 {
-    if (OBJ_FROZEN(str)) {
-       return str;
-    }
-    else {
-       return rb_fstring(str);
-    }
+    return rb_fstring(str);
 }
```

But this seems to break the test/ruby/test_string.rb cause there is some edge case we are missing here. 


----------------------------------------
Feature #14478: String #uminus should de-dupe unconditionally
https://bugs.ruby-lang.org/issues/14478#change-70699

* Author: sam.saffron (Sam Saffron)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
continuing: https://bugs.ruby-lang.org/issues/14475

Current documentation for String uminus says: "If the string is frozen, then return the string itself."

Trouble is that there is no simple way to de-duplicate unconditionally without inefficiency:

Say `x` is an arbitrary string (either frozen or unfrozen) :

```
x = -x # may return a non fstring

x = -+x # will return fstring, but makes an unneeded copy

x = -x.dup # fstring again, uneeded copy 

x = x.frozen? ? -+x : -x # too verbose, uneeded copy
```

Instead why not change it so `-` is deduped unconditionally? 

I would argue this is worth backporting, cause if we are making fstring optimisations now, we are going to be stuck with legacy inefficient code going forward.

An alternative may be a c-extension gem that adds #fstring to String but that just feel wrong. 

I think the documentation should say:

String uminus says: "If the string is de-duplicated, then return the string itself."

Happy to make the change it is quite simple:`(FL_TEST(str, RSTRING_FSTR)` 






-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>