[pull] master from ruby:master

.github/auto_request_review.yml

@@ -11,6 +11,7 @@ files:
   'doc/zjit*': [team:jit]
   'test/ruby/test_zjit*': [team:jit]
   'defs/jit.mk': [team:jit]
+  # Skip github workflow files because the team don't necessarily need to review dependabot updates for GitHub Actions. It's noisy in notifications, and they're auto-merged anyway.
 options:
   ignore_draft: true
   # This currently doesn't work as intended. We want to skip reviews when only

.github/workflows/yjit-macos.yml

@@ -145,6 +145,7 @@ jobs:
           test -n "${LAUNCHABLE_STDOUT}" && exec 1> >(tee "${LAUNCHABLE_STDOUT}")
           test -n "${LAUNCHABLE_STDERR}" && exec 2> >(tee "${LAUNCHABLE_STDERR}")
 
+          set -x
           make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} \
             RUN_OPTS="$RUN_OPTS" \
             SPECOPTS="$SPECOPTS"

.github/workflows/yjit-ubuntu.yml

@@ -195,6 +195,7 @@ jobs:
           test -n "${LAUNCHABLE_STDOUT}" && exec 1> >(tee "${LAUNCHABLE_STDOUT}")
           test -n "${LAUNCHABLE_STDERR}" && exec 2> >(tee "${LAUNCHABLE_STDERR}")
 
+          set -x
           make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} \
             RUN_OPTS="$RUN_OPTS" MSPECOPT=--debug SPECOPTS="$SPECOPTS" \
             YJIT_BENCH_OPTS="$YJIT_BENCH_OPTS" YJIT_BINDGEN_DIFF_OPTS="$YJIT_BINDGEN_DIFF_OPTS"

.github/workflows/zjit-macos.yml

@@ -33,9 +33,15 @@ jobs:
       matrix:
         include:
           - test_task: 'check'
-            configure: '--enable-zjit=dev'
             run_opts: '--zjit-call-threshold=1'
             specopts: '-T --zjit-call-threshold=1'
+            configure: '--enable-zjit=dev'
+            test_all_opts: '--seed=46450'
+
+          - test_task: 'check'
+            run_opts: '--zjit-call-threshold=1 --zjit-disable-hir-opt'
+            specopts: '-T --zjit-call-threshold=1 -T --zjit-disable-hir-opt'
+            configure: '--enable-zjit=dev'
             test_all_opts: '--seed=46450'
 
           - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb
@@ -112,11 +118,12 @@ jobs:
           echo "RUBY_CRASH_REPORT=$(pwd)/rb_crash_%p.txt" >> $GITHUB_ENV
 
       - name: make ${{ matrix.test_task }}
-        run: >-
-          make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"}
-          RUN_OPTS="$RUN_OPTS"
-          SPECOPTS="$SPECOPTS"
-          TESTOPTS="$TESTOPTS"
+        run: |
+          set -x
+          make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} \
+            RUN_OPTS="$RUN_OPTS" \
+            SPECOPTS="$SPECOPTS" \
+            TESTOPTS="$TESTOPTS"
         timeout-minutes: 60
         env:
           RUBY_TESTOPTS: '-q --tty=no'

.github/workflows/zjit-ubuntu.yml

@@ -52,9 +52,15 @@ jobs:
       matrix:
         include:
           - test_task: 'check'
-            configure: '--enable-zjit=dev'
             run_opts: '--zjit-call-threshold=1'
             specopts: '-T --zjit-call-threshold=1'
+            configure: '--enable-zjit=dev'
+            test_all_opts: '--seed=39471'
+
+          - test_task: 'check'
+            run_opts: '--zjit-call-threshold=1 --zjit-disable-hir-opt'
+            specopts: '-T --zjit-call-threshold=1 -T --zjit-disable-hir-opt'
+            configure: '--enable-zjit=dev'
             test_all_opts: '--seed=39471'
 
           - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb
@@ -152,11 +158,12 @@ jobs:
           echo "RUBY_CRASH_REPORT=$(pwd)/rb_crash_%p.txt" >> $GITHUB_ENV
 
       - name: make ${{ matrix.test_task }}
-        run: >-
-          make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"}
-          RUN_OPTS="$RUN_OPTS" MSPECOPT=--debug SPECOPTS="$SPECOPTS"
-          TESTOPTS="$TESTOPTS"
-          ZJIT_BINDGEN_DIFF_OPTS="$ZJIT_BINDGEN_DIFF_OPTS"
+        run: |
+          set -x
+          make -s ${{ matrix.test_task }} ${TESTS:+TESTS="$TESTS"} \
+            RUN_OPTS="$RUN_OPTS" MSPECOPT=--debug SPECOPTS="$SPECOPTS" \
+            TESTOPTS="$TESTOPTS" \
+            ZJIT_BINDGEN_DIFF_OPTS="$ZJIT_BINDGEN_DIFF_OPTS"
         timeout-minutes: 90
         env:
           RUBY_TESTOPTS: '-q --tty=no'

encoding.c

@@ -152,6 +152,8 @@ enc_list_update(int index, rb_raw_encoding *encoding)
         RBASIC_CLEAR_CLASS(new_list);
         /* initialize encoding data */
         rb_ary_store(new_list, index, enc_new(encoding));
+        rb_ary_freeze(new_list);
+        FL_SET_RAW(new_list, RUBY_FL_SHAREABLE);
         RUBY_ATOMIC_VALUE_SET(rb_encoding_list, new_list);
     }
 }
@@ -349,6 +351,35 @@ enc_table_expand(struct enc_table *enc_table, int newsize)
     return newsize;
 }
 
+/* Load an encoding using the values from base_encoding */
+static void
+enc_load_from_base(struct enc_table *enc_table, int index, rb_encoding *base_encoding)
+{
+    ASSERT_vm_locking();
+
+    struct rb_encoding_entry *ent = &enc_table->list[index];
+
+    if (ent->loaded) {
+        return;
+    }
+
+    rb_raw_encoding *encoding = (rb_raw_encoding *)ent->enc;
+    RUBY_ASSERT(encoding);
+
+    // FIXME: Before the base is loaded, the encoding may be accessed
+    // concurrently by other Ractors.
+    // We're copying all fields from base_encoding except name and
+    // ruby_encoding_index which we preserve from the original. Since these are
+    // the only fields other threads should read it is likely safe despite
+    // technically being a data race.
+    rb_raw_encoding tmp_encoding = *base_encoding;
+    tmp_encoding.name = encoding->name;
+    tmp_encoding.ruby_encoding_index = encoding->ruby_encoding_index;
+    *encoding = tmp_encoding;
+
+    RUBY_ATOMIC_SET(ent->loaded, encoding->max_enc_len);
+}
+
 static int
 enc_register_at(struct enc_table *enc_table, int index, const char *name, rb_encoding *base_encoding)
 {
@@ -357,45 +388,33 @@ enc_register_at(struct enc_table *enc_table, int index, const char *name, rb_enc
     struct rb_encoding_entry *ent = &enc_table->list[index];
     rb_raw_encoding *encoding;
 
-    if (ent->loaded) {
-        RUBY_ASSERT(ent->base == base_encoding);
-        RUBY_ASSERT(!strcmp(name, ent->name));
-        return index;
-    }
+    RUBY_ASSERT(!ent->loaded);
+    RUBY_ASSERT(!ent->name);
+    RUBY_ASSERT(!ent->enc);
+    RUBY_ASSERT(!ent->base);
 
-    if (!valid_encoding_name_p(name)) return -1;
-    if (!ent->name) {
-        ent->name = name = strdup(name);
-    }
-    else if (STRCASECMP(name, ent->name)) {
-        return -1;
-    }
-    encoding = (rb_raw_encoding *)ent->enc;
-    if (!encoding) {
-        encoding = xmalloc(sizeof(rb_encoding));
-    }
+    RUBY_ASSERT(valid_encoding_name_p(name));
 
-    rb_raw_encoding tmp_encoding;
-    if (base_encoding) {
-        tmp_encoding = *base_encoding;
-    }
-    else {
-        memset(&tmp_encoding, 0, sizeof(*ent->enc));
-    }
-    tmp_encoding.name = name;
-    tmp_encoding.ruby_encoding_index = index;
+    ent->name = name = strdup(name);
 
-    // FIXME: If encoding already existed, it may be concurrently accessed
-    // It's technically invalid to write to this memory as it's read, but as all
-    // values are set up it _probably_ works.
-    *encoding = tmp_encoding;
+    encoding = ZALLOC(rb_raw_encoding);
+    encoding->name = name;
+    encoding->ruby_encoding_index = index;
     ent->enc = encoding;
-    st_insert(enc_table->names, (st_data_t)name, (st_data_t)index);
+
+    if (st_insert(enc_table->names, (st_data_t)name, (st_data_t)index)) {
+        rb_bug("encoding name was somehow registered twice");
+    }
 
     enc_list_update(index, encoding);
 
-    // max_enc_len is used to mark a fully loaded encoding.
-    RUBY_ATOMIC_SET(ent->loaded, encoding->max_enc_len);
+    if (base_encoding) {
+        enc_load_from_base(enc_table, index, base_encoding);
+    }
+    else {
+        /* it should not be loaded yet */
+        RUBY_ASSERT(!encoding->max_enc_len);
+    }
 
     return index;
 }
@@ -405,6 +424,8 @@ enc_register(struct enc_table *enc_table, const char *name, rb_encoding *encodin
 {
     ASSERT_vm_locking();
 
+    if (!valid_encoding_name_p(name)) return -1;
+
     int index = enc_table->count;
 
     enc_table->count = enc_table_expand(enc_table, index + 1);
@@ -445,7 +466,7 @@ rb_enc_register(const char *name, rb_encoding *encoding)
                 index = enc_register(enc_table, name, encoding);
             }
             else if (rb_enc_autoload_p(oldenc) || !ENC_DUMMY_P(oldenc)) {
-                enc_register_at(enc_table, index, name, encoding);
+                enc_load_from_base(enc_table, index, encoding);
             }
             else {
                 rb_raise(rb_eArgError, "encoding %s is already registered", name);
@@ -562,7 +583,7 @@ enc_replicate_with_index(struct enc_table *enc_table, const char *name, rb_encod
         idx = enc_register(enc_table, name, origenc);
     }
     else {
-        idx = enc_register_at(enc_table, idx, name, origenc);
+        enc_load_from_base(enc_table, idx, origenc);
     }
     if (idx >= 0) {
         set_base_encoding(enc_table, idx, origenc);
@@ -816,41 +837,28 @@ enc_autoload_body(rb_encoding *enc)
 
     GLOBAL_ENC_TABLE_LOCKING(enc_table) {
         base = enc_table->list[ENC_TO_ENCINDEX(enc)].base;
-        if (base) {
-            do {
-                if (i >= enc_table->count) {
-                    i = -1;
-                    break;
-                }
-            } while (enc_table->list[i].enc != base && (++i, 1));
-        }
     }
 
-
-    if (i != -1) {
-        if (base) {
-            bool do_register = true;
-            if (rb_enc_autoload_p(base)) {
-                if (rb_enc_autoload(base) < 0) {
-                    do_register = false;
-                    i = -1;
-                }
+    if (base) {
+        bool do_register = true;
+        if (rb_enc_autoload_p(base)) {
+            if (rb_enc_autoload(base) < 0) {
+                do_register = false;
+                i = -1;
             }
+        }
 
-            if (do_register) {
-                GLOBAL_ENC_TABLE_LOCKING(enc_table) {
-                    i = enc->ruby_encoding_index;
-                    enc_register_at(enc_table, i & ENC_INDEX_MASK, rb_enc_name(enc), base);
-                    RUBY_ASSERT(((rb_raw_encoding *)enc)->ruby_encoding_index == i);
-                }
+        if (do_register) {
+            GLOBAL_ENC_TABLE_LOCKING(enc_table) {
+                i = ENC_TO_ENCINDEX(enc);
+                enc_load_from_base(enc_table, i, base);
+                RUBY_ASSERT(((rb_raw_encoding *)enc)->ruby_encoding_index == i);
             }
-
-            i &= ENC_INDEX_MASK;
-        }
-        else {
-            i = -2;
         }
     }
+    else {
+        i = -2;
+    }
 
     return i;
 }

imemo.c

@@ -153,7 +153,7 @@ imemo_fields_complex_wb_i(st_data_t key, st_data_t value, st_data_t arg)
 VALUE
 rb_imemo_fields_new_complex_tbl(VALUE owner, st_table *tbl)
 {
-    VALUE fields = imemo_fields_new(owner, sizeof(struct rb_fields));
+    VALUE fields = rb_imemo_new(imemo_fields, owner, sizeof(struct rb_fields));
     IMEMO_OBJ_FIELDS(fields)->as.complex.table = tbl;
     FL_SET_RAW(fields, OBJ_FIELD_HEAP);
     st_foreach(tbl, imemo_fields_trigger_wb_i, (st_data_t)fields);

prism/prism.c

@@ -14443,6 +14443,17 @@ parse_arguments(pm_parser_t *parser, pm_arguments_t *arguments, bool accepts_for
             if (accepted_newline) {
                 pm_parser_err_previous(parser, PM_ERR_INVALID_COMMA);
             }
+
+            // If this is a command call and an argument takes a block,
+            // there can be no further arguments. For example,
+            // `foo(bar 1 do end, 2)` should be rejected.
+            if (PM_NODE_TYPE_P(argument, PM_CALL_NODE)) {
+                pm_call_node_t *call = (pm_call_node_t *) argument;
+                if (call->opening_loc.start == NULL && call->arguments != NULL && call->block != NULL) {
+                    pm_parser_err_previous(parser, PM_ERR_INVALID_COMMA);
+                    break;
+                }
+            }
         } else {
             // If there is no comma at the end of the argument list then we're
             // done parsing arguments and can break out of this loop.

prism/util/pm_string.c

@@ -1,5 +1,7 @@
 #include "prism/util/pm_string.h"
 
+static const uint8_t empty_source[] = "";
+
 /**
  * Returns the size of the pm_string_t struct. This is necessary to allocate the
  * correct amount of memory in the FFI backend.
@@ -133,8 +135,7 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
     // the source to a constant empty string and return.
     if (file_size == 0) {
         pm_string_file_handle_close(&handle);
-        const uint8_t source[] = "";
-        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
+        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = empty_source, .length = 0 };
         return PM_STRING_INIT_SUCCESS;
     }
 
@@ -182,8 +183,7 @@ pm_string_mapped_init(pm_string_t *string, const char *filepath) {
 
     if (size == 0) {
         close(fd);
-        static const uint8_t source[] = "";
-        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
+        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = empty_source, .length = 0 };
         return PM_STRING_INIT_SUCCESS;
     }
 
@@ -225,8 +225,7 @@ pm_string_file_init(pm_string_t *string, const char *filepath) {
     // the source to a constant empty string and return.
     if (file_size == 0) {
         pm_string_file_handle_close(&handle);
-        const uint8_t source[] = "";
-        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
+        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = empty_source, .length = 0 };
         return PM_STRING_INIT_SUCCESS;
     }
 
@@ -278,8 +277,7 @@ pm_string_file_init(pm_string_t *string, const char *filepath) {
     size_t size = (size_t) sb.st_size;
     if (size == 0) {
         close(fd);
-        static const uint8_t source[] = "";
-        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = source, .length = 0 };
+        *string = (pm_string_t) { .type = PM_STRING_CONSTANT, .source = empty_source, .length = 0 };
         return PM_STRING_INIT_SUCCESS;
     }
 

test/prism/errors/command_calls.txt

@@ -1,3 +1,10 @@
 [a b]
   ^ unexpected local variable or method; expected a `,` separator for the array elements
 
+
+[
+  a b do
+   ^ unexpected local variable or method; expected a `,` separator for the array elements
+  end,
+]
+

test/prism/errors/command_calls_34.txt

@@ -0,0 +1,24 @@
+foo(bar 1 do end, 2)
+                ^ invalid comma
+                  ^ unexpected integer; expected a `)` to close the arguments
+                  ^ unexpected integer, expecting end-of-input
+                   ^ unexpected ')', expecting end-of-input
+                   ^ unexpected ')', ignoring it
+
+foo(bar 1 do end,)
+                ^ invalid comma
+
+foo(1, bar 2 do end)
+           ^ unexpected integer; expected a `)` to close the arguments
+           ^ unexpected integer, expecting end-of-input
+             ^~ unexpected 'do', expecting end-of-input
+             ^~ unexpected 'do', ignoring it
+                ^~~ unexpected 'end', ignoring it
+                   ^ unexpected ')', ignoring it
+
+foo(1, bar 2)
+           ^ unexpected integer; expected a `)` to close the arguments
+           ^ unexpected integer, expecting end-of-input
+            ^ unexpected ')', expecting end-of-input
+            ^ unexpected ')', ignoring it
+