diff --git a/Makefile b/Makefile
index 5500ddbd6c..e221ca0369 100644
--- a/Makefile
+++ b/Makefile
@@ -1385,7 +1385,7 @@ link-names-test: $(SPINEL) $(SP_RT_LIB) $(SP_RT_MT_LIB)
reject-test: $(SPINEL)
@ok=1; tmp=$$(mktemp -d /tmp/spinel-reject.XXXXXX); \
- for t in test/reject/string_thread_arg.rb test/reject/string_fiber_arg.rb test/reject/string_thread_global_arg.rb test/reject/string_global_hash_element_mutation.rb test/reject/string_thread_ivar_arg.rb test/reject/string_thread_method_param_arg.rb test/reject/string_fiber_method_param_arg.rb test/reject/string_thread_block_param_arg.rb test/reject/string_thread_arg_in_loop.rb; do \
+ for t in test/reject/string_thread_arg.rb test/reject/string_fiber_arg.rb test/reject/string_thread_global_arg.rb test/reject/string_global_hash_element_mutation.rb test/reject/string_thread_ivar_arg.rb test/reject/string_thread_method_param_arg.rb test/reject/string_fiber_method_param_arg.rb test/reject/string_thread_block_param_arg.rb test/reject/string_thread_arg_in_loop.rb test/reject/string_split_local_each_mutation.rb test/reject/string_split_select_mutation.rb test/reject/string_split_map_bang_read_before.rb; do \
if $(SPINEL) "$$t" -c --no-line-map -o "$$tmp/sk.c" >"$$tmp/sk.out" 2>&1; then \
echo "reject-test: FAIL ($$t compiled)"; ok=0; \
else grep -q "is not yet shared by reference" "$$tmp/sk.out" || \
diff --git a/src/analyze.c b/src/analyze.c
index 5f70dfe333..dea64e5604 100644
--- a/src/analyze.c
+++ b/src/analyze.c
@@ -18490,6 +18490,136 @@ static void an_mark_handle_returns(Compiler *c) {
if (poly) c->strbuf_box[n] = 0;
}
}
+/* A String builtin answering a new Array of new Strings nothing else holds
+ (`s.split(",")`, `s.scan(re)`, `s.lines`, `s.chars`). */
+static int sb_fresh_string_array_call(Compiler *c, int n) {
+ const NodeTable *nt = c->nt;
+ n = an_unparen(nt, n);
+ if (n < 0 || nt_kind(nt, n) != NK_CallNode || nt_ref(nt, n, "block") >= 0) return 0;
+ const char *mn = nt_str(nt, n, "name");
+ int r = nt_ref(nt, n, "receiver");
+ if (!mn || r < 0) return 0;
+ if (!sp_streq(mn, "split") && !sp_streq(mn, "scan") && !sp_streq(mn, "lines") && !sp_streq(mn, "chars"))
+ return 0;
+ TyKind rt = infer_type(c, r);
+ return (rt == TY_STRING || rt == TY_STRBUF) && infer_type(c, n) == TY_STR_ARRAY;
+}
+static int sb_subtree_max(const NodeTable *nt, int n) {
+ if (n < 0) return -1;
+ int m = n;
+ const SpNode *nd = &nt->nodes[n];
+ for (int i = 0; i < nd->nr; i++) { int x = sb_subtree_max(nt, nd->r[i].ref); if (x > m) m = x; }
+ for (int i = 0; i < nd->na; i++)
+ for (int j = 0; j < nd->a[i].n; j++) { int x = sb_subtree_max(nt, nd->a[i].ids[j]); if (x > m) m = x; }
+ return m;
+}
+/* map!/collect! over local `vn` (call w) mutating each element it yields:
+ the elements are replaced by the block's values, so a mutation of the
+ yielded String is seen only where the block's value carries it -- unless
+ the String is reachable some other way. It is not when the local is only
+ ever bound to a fresh String Array (sb_fresh_string_array_call) ahead of
+ w, every other read of it comes after w (none can hand an element out
+ before the replacement), and no loop around w runs it again over Strings
+ a later read may have handed out. */
+static int sb_map_bang_local_unobserved(Compiler *c, int w, int recv, const char *vn, Scope *vs) {
+ const NodeTable *nt = c->nt;
+ if (!vs || vs->body < 0 || !an_subtree_has(nt, vs->body, w)) return 0;
+ int wmax = sb_subtree_max(nt, w), writes = 0;
+ for (int u = comp_kind_first(c, NK_LocalVariableWriteNode); u >= 0; u = comp_kind_next(c, u)) {
+ if (nt_kind(nt, u) != NK_LocalVariableWriteNode || !sp_streq(nt_str(nt, u, "name"), vn)) continue;
+ if (!an_subtree_has(nt, vs->body, u)) continue;
+ if (u > w || !sb_fresh_string_array_call(c, nt_ref(nt, u, "value"))) return 0;
+ writes++;
+ }
+ if (writes == 0) return 0;
+ NodeKind other[] = { NK_LocalVariableOrWriteNode, NK_LocalVariableAndWriteNode,
+ NK_LocalVariableOperatorWriteNode, NK_LocalVariableTargetNode };
+ for (int k = 0; k < (int)(sizeof other / sizeof other[0]); k++)
+ for (int u = comp_kind_first(c, other[k]); u >= 0; u = comp_kind_next(c, u))
+ if (nt_kind(nt, u) == other[k] && sp_streq(nt_str(nt, u, "name"), vn) &&
+ an_subtree_has(nt, vs->body, u)) return 0;
+ for (int u = comp_kind_first(c, NK_LocalVariableReadNode); u >= 0; u = comp_kind_next(c, u)) {
+ if (u == recv || nt_kind(nt, u) != NK_LocalVariableReadNode || !sp_streq(nt_str(nt, u, "name"), vn)) continue;
+ if (u <= wmax && an_subtree_has(nt, vs->body, u)) return 0;
+ }
+ NodeKind loops[] = { NK_WhileNode, NK_UntilNode, NK_ForNode };
+ for (int k = 0; k < 3; k++)
+ for (int u = comp_kind_first(c, loops[k]); u >= 0; u = comp_kind_next(c, u))
+ if (nt_kind(nt, u) == loops[k] && an_subtree_has(nt, u, w)) return 0;
+ /* w runs where the local lives, not in a block that may run it again */
+ return comp_scope_of(c, w) == vs;
+}
+/* Does the subtree at n hold a node that leaves a block iteration early
+ (next, redo, retry) or writes local `pn`? */
+static int sb_block_body_blocks_rewrite(const NodeTable *nt, int n, const char *pn) {
+ if (n < 0) return 0;
+ NodeKind k = nt_kind(nt, n);
+ if (k == NK_NextNode || k == NK_RedoNode || k == NK_RetryNode) return 1;
+ if ((k == NK_LocalVariableWriteNode || k == NK_LocalVariableOrWriteNode ||
+ k == NK_LocalVariableAndWriteNode || k == NK_LocalVariableOperatorWriteNode ||
+ k == NK_LocalVariableTargetNode) && sp_streq(nt_str(nt, n, "name"), pn)) return 1;
+ const SpNode *nd = &nt->nodes[n];
+ for (int i = 0; i < nd->nr; i++)
+ if (sb_block_body_blocks_rewrite(nt, nd->r[i].ref, pn)) return 1;
+ for (int i = 0; i < nd->na; i++)
+ for (int j = 0; j < nd->a[i].n; j++)
+ if (sb_block_body_blocks_rewrite(nt, nd->a[i].ids[j], pn)) return 1;
+ return 0;
+}
+/* `s.scan(re).each { |x| x.gsub!(...) }.join`: each answers the fresh Array
+ it walked, and its answer is read, so the in-place mutation of each
+ element must be seen there -- but a String Array's element binds the
+ parameter as a copy. Nothing else holds that Array or its Strings, so
+ storing the parameter's final value back over its element is the same
+ program: the block becomes `{ |x| ...; x }` under map!, which answers the
+ Array too. Only a block that runs to its end every time (no next, redo or
+ retry) and never rebinds the parameter; a break answers the break value
+ either way and leaves the Array unread. */
+static int desugar_fresh_array_each_writeback(Compiler *c) {
+ NodeTable *nt = (NodeTable *)c->nt;
+ int n0 = nt->count, changed = 0;
+ for (int w = comp_kind_first(c, NK_CallNode); w >= 0 && w < n0; w = comp_kind_next(c, w)) {
+ if (nt_kind(nt, w) != NK_CallNode || !sp_streq(nt_str(nt, w, "name"), "each")) continue;
+ int blk = nt_ref(nt, w, "block"), recv = nt_ref(nt, w, "receiver");
+ if (blk < 0 || nt_kind(nt, blk) != NK_BlockNode || nt_ref(nt, w, "arguments") >= 0) continue;
+ if (comp_value_dropped(c, w) || !sb_fresh_string_array_call(c, recv)) continue;
+ int bp = nt_ref(nt, blk, "parameters");
+ if (bp < 0 || nt_kind(nt, bp) != NK_BlockParametersNode) continue;
+ int pn = nt_ref(nt, bp, "parameters");
+ if (pn < 0) continue;
+ int nreq = 0, nopt = 0, npost = 0, nkw = 0;
+ const int *req = nt_arr(nt, pn, "requireds", &nreq);
+ nt_arr(nt, pn, "optionals", &nopt); nt_arr(nt, pn, "posts", &npost); nt_arr(nt, pn, "keywords", &nkw);
+ if (nreq != 1 || nopt || npost || nkw || nt_ref(nt, pn, "rest") >= 0 ||
+ nt_ref(nt, pn, "keyword_rest") >= 0 || nt_ref(nt, pn, "block") >= 0 ||
+ nt_kind(nt, req[0]) != NK_RequiredParameterNode) continue;
+ const char *pname = nt_str(nt, req[0], "name");
+ Scope *bs = comp_scope_of(c, blk);
+ int body = nt_ref(nt, blk, "body");
+ if (!pname || !bs || body < 0 || nt_kind(nt, body) != NK_StatementsNode) continue;
+ if (strbuf_mut_kind(c, pname, bs) != 1 || sb_block_body_blocks_rewrite(nt, body, pname)) continue;
+ int bn = 0; const int *bb = nt_arr(nt, body, "body", &bn);
+ int base = nt->count;
+ int rd = nt_new_node(nt, "LocalVariableReadNode");
+ if (rd < 0) continue;
+ nt_node_set_str(nt, rd, "name", pname);
+ nt_node_set_int(nt, rd, "depth", 0);
+ int *nb = malloc(sizeof(int) * (size_t)(bn + 1));
+ if (!nb) { fprintf(stderr, "spinel: out of memory\n"); exit(1); }
+ memcpy(nb, bb, sizeof(int) * (size_t)bn);
+ nb[bn] = rd;
+ int st = nt_new_node(nt, "StatementsNode");
+ if (st < 0) { free(nb); continue; }
+ nt_node_set_arr(nt, st, "body", nb, bn + 1);
+ free(nb);
+ nt_node_set_ref(nt, blk, "body", st);
+ nt_set_str(nt, w, "name", "map!");
+ comp_grow_node_arrays(c);
+ for (int j = base; j < nt->count; j++) c->nscope[j] = c->nscope[blk];
+ changed = 1;
+ }
+ return changed;
+}
static int promote_shared_stored_strings(Compiler *c) {
int changed = 0;
/* --share-strings: the one rule decides first (#6765) */
@@ -19173,6 +19303,20 @@ static int promote_shared_stored_strings(Compiler *c) {
for (int e = 0; e < en && !var; e++) var = nt_kind(nt, el[e]) == NK_LocalVariableReadNode;
if (!var) continue;
}
+ /* A fresh String Array a call answers (`s.split(",").map! { |t| t.strip!
+ ... }`) that nothing else holds: a mutation of the String an iterator
+ yields is seen only through the block's own value when the
+ iterator's answer is dropped, or is the block's values (map, and
+ map!/collect!, which store them over the elements). An iterator
+ answering the Array or some of its elements (each, select, find) is
+ read again where its answer is used, and stays refused here
+ (desugar_fresh_array_each_writeback takes each's). The parameter then
+ binds the element as a copy, as for any read. */
+ if ((bpv4->type == TY_STRING || bpv4->type == TY_STRBUF) && sb_fresh_string_array_call(c, recv4) &&
+ (is_map_bang_alias(itn) || comp_value_dropped(c, w) ||
+ bop_share_named(BOP_ANY_ARRAY, itn) == BSH_ITER_MAP ||
+ bop_share_named(BOP_ANY_ARRAY, itn) == BSH_ITER_MAP_BANG))
+ continue;
if ((bpv4->type == TY_STRING || bpv4->type == TY_STRBUF) &&
(nt_kind(nt, recv4) == NK_InstanceVariableReadNode || nt_kind(nt, recv4) == NK_CallNode) &&
(infer_type(c, recv4) == TY_STR_ARRAY || infer_type(c, recv4) == TY_POLY_ARRAY)) {
@@ -19226,6 +19370,12 @@ static int promote_shared_stored_strings(Compiler *c) {
(settled or literal) element type so an array-of-arrays each+<<
never promotes its param */
if (contt4 != TY_STR_ARRAY && contt4 != TY_POLY_ARRAY) continue;
+ /* map!/collect! over a local only ever bound to a fresh String Array,
+ read nowhere before the replacement: the block's value is what each
+ element becomes, and the mutated String is reachable only through it */
+ if (!lit4 && contt4 == TY_STR_ARRAY && is_map_bang_alias(itn) &&
+ sb_map_bang_local_unobserved(c, w, recv4, contn4, conts4))
+ continue;
if (!lit4 && contt4 == TY_STR_ARRAY &&
an_local_string_array_has_untracked_call_store(c, contn4, conts4)) {
static const char sa_msg[] =
@@ -34649,6 +34799,7 @@ static void an_phase_infer_fixpoint(Compiler *c) {
ch |= desugar_enum_iter_splat_args(c); /* enum.map(*a, &b) -> enum.map(&b) */
ch |= desugar_enum_pair_lone_param(c); /* a.each_with_index.map { |x| } -> { |x, i| } */
ch |= desugar_builtin_iter_block_shapes(c); /* [1].each { |c, a = 10| } -> { |v| c = v; a = 10 } */
+ ch |= desugar_fresh_array_each_writeback(c); /* s.scan(re).each { |x| x.gsub!(..) }.join -> map! { |x| ..; x } */
ch |= desugar_multi_yield_map_param(c); /* multi-yield each: map's |x| takes the 1st */
ch |= desugar_ewi_pack_values(c); /* multi-yield each: each_with_index packs */
ch |= desugar_enum_walk_calls(c); /* enum.map { break } -> __enumw_map(enum) { } */
diff --git a/test/fresh_string_array_element_mutation.rb b/test/fresh_string_array_element_mutation.rb
new file mode 100644
index 0000000000..c6ed015502
--- /dev/null
+++ b/test/fresh_string_array_element_mutation.rb
@@ -0,0 +1,45 @@
+# A String yielded out of a fresh Array (split, scan) and mutated in place
+# where nothing can read the mutation back through that Array: the Array is
+# a call's temporary whose iterator answer is dropped or is the block's
+# values, or a map!/collect! stores each block value over its element.
+# These were refused ("not yet shared by reference") although a copy gives
+# the same answer. Actionview's highlight and split_paragraphs and rack's
+# parse_http_accept_header are written so.
+
+# highlight: each answers the scanned Array and join reads it, so each
+# mutated element is stored back (each { |x| ...; } runs as map! { |x| ...; x })
+def highlight(text)
+ text.scan(/<[^>]*|[^<]+/).each do |segment|
+ if !segment.start_with?("<")
+ segment.gsub!(/b/, "B")
+ end
+ end.join
+end
+p highlight("abcb")
+
+# split_paragraphs: map! keeps the block's value
+def split_paragraphs(text)
+ text.to_str.gsub(/\r\n?/, "\n").split(/\n\n+/).map! { |t| t.gsub!(/([^\n]\n)(?=[^\n])/, '\1
') || t }
+end
+p split_paragraphs("a\nb\n\nc")
+
+# parse_http_accept_header: a local bound to split and read only after map!
+def accept(header)
+ parts = header.to_s.split(",")
+ parts.map! { |part|
+ part.strip!
+ next if part.empty?
+ part.downcase
+ }
+ parts.compact!
+ parts
+end
+p accept("text/html, ,Application/JSON ")
+
+s = " a , b ,c "
+s.split(",").each { |x| x.strip!; print x, ";" }
+puts
+p s.split(",").map { |x| x.strip!; x.upcase }
+p s.split(",").each { |x| x.strip! }
+p "x\ny\n".lines.each(&:chomp!)
+p s
diff --git a/test/fresh_string_array_element_mutation.rb.expected b/test/fresh_string_array_element_mutation.rb.expected
new file mode 100644
index 0000000000..0f0ce11022
--- /dev/null
+++ b/test/fresh_string_array_element_mutation.rb.expected
@@ -0,0 +1,8 @@
+"aBcB"
+["a\n
b", "c"]
+["text/html", "application/json"]
+a;b;c;
+["A", "B", "C"]
+["a", "b", "c"]
+["x", "y"]
+" a , b ,c "
diff --git a/test/reject/string_split_local_each_mutation.rb b/test/reject/string_split_local_each_mutation.rb
new file mode 100644
index 0000000000..1a1b1f910f
--- /dev/null
+++ b/test/reject/string_split_local_each_mutation.rb
@@ -0,0 +1,7 @@
+# A local bound to split's Array whose Strings an each block strips in
+# place, and that is read afterwards: each keeps the elements, so the
+# mutation must be seen through the Array. Still refused, not silently
+# applied to a copy.
+a = " x , y ".split(",")
+a.each(&:strip!)
+p a
diff --git a/test/reject/string_split_map_bang_read_before.rb b/test/reject/string_split_map_bang_read_before.rb
new file mode 100644
index 0000000000..4421a42fd7
--- /dev/null
+++ b/test/reject/string_split_map_bang_read_before.rb
@@ -0,0 +1,7 @@
+# map! over a local whose element was read out before the replacement: the
+# String it handed out is the one map!'s block mutates in place. Refused,
+# not silently applied to a copy.
+parts = " a , b ".split(",")
+first = parts[0]
+parts.map! { |x| x.strip! || x }
+p first, parts
diff --git a/test/reject/string_split_select_mutation.rb b/test/reject/string_split_select_mutation.rb
new file mode 100644
index 0000000000..ec4576c94c
--- /dev/null
+++ b/test/reject/string_split_select_mutation.rb
@@ -0,0 +1,4 @@
+# select answers some of the very Strings its block mutated in place, so
+# the mutation is read back through its answer: refused, not silently
+# applied to a copy.
+p " a , b ".split(",").select { |x| x.strip!; true }