From 7ab8e16b2b4c85f804f212fa9e20ac5f0f5c4ec9 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 11 Sep 2025 20:29:12 +0900 Subject: [PATCH 1/5] Check CAPI ext binary compatibility --- .github/actions/capiext/action.yml | 44 ++++++++++++++++++++++++++++++ .github/workflows/macos.yml | 14 ++++++++++ .github/workflows/ubuntu.yml | 14 ++++++++++ 3 files changed, 72 insertions(+) create mode 100644 .github/actions/capiext/action.yml diff --git a/.github/actions/capiext/action.yml b/.github/actions/capiext/action.yml new file mode 100644 index 00000000000000..4958775707a94d --- /dev/null +++ b/.github/actions/capiext/action.yml @@ -0,0 +1,44 @@ +name: rubyspec C-API extensions + +inputs: + builddir: + required: false + default: '.' + +outputs: + key: + value: >- + ${{ + !steps.restore.outputs.cache-hit && + github.ref == 'refs/heads/master' && + steps.config.outputs.key + }} + +runs: + using: composite + + steps: + - id: config + shell: bash + run: | + eval $(grep -e '^arch *=' -e '^ruby_version *=' -e '^DLEXT *=' Makefile | + sed 's/ *= */=/') + key=capiexts-${arch}-${ruby_version} + echo key=$key >> $GITHUB_OUTPUT + echo DLEXT=$DLEXT >> $GITHUB_OUTPUT + working-directory: ${{ inputs.builddir }} + + - name: CAPI extensions cache + uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4.2.4 + id: cache + with: + path: ${{ inputs.builddir }}/spec/ruby/optional/capi/ext/ + key: ${{ steps.config.outputs.key }} + + - name: Force rubyspec CAPI extension binaries up to date + shell: bash + run: | + touch ${{ inputs.builddir }}/spec/ruby/optional/capi/ext/*.$DLEXT + env: + DLEXT: ${{ steps.config.outputs.DLEXT }} + if: ${{ steps.cache.outputs.cache-hit }} diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 1f96c16c7107e8..9205120e2a1bbb 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -45,6 +45,7 @@ jobs: os: macos-14 - test_task: check os: macos-15 + capi_check: true - test_task: check os: macos-13 fail-fast: false @@ -159,6 +160,19 @@ jobs: if: ${{ matrix.test_task == 'check' && matrix.skipped_tests }} continue-on-error: ${{ matrix.continue-on-skipped_tests || false }} + - name: CAPI extensions + uses: ./.github/actions/capiext + with: + builddir: build + if: ${{ matrix.capi_check }} + + - name: Check with previous CAPI extensions + run: | + make -s test-spec + env: + RUBY_TESTOPTS: '-v --tty=no' + if: ${{ matrix.capi_check }} + - uses: ./.github/actions/slack with: label: ${{ matrix.os }} / ${{ matrix.test_task }} diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index fa799ebd39a2f9..2ce6a5728c04dd 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -39,6 +39,7 @@ jobs: - test_task: test-bundled-gems - test_task: check os: ubuntu-24.04 + capi_check: true # ubuntu-24.04-arm jobs don't start on ruby/ruby as of 2025-09-04 #- test_task: check # os: ubuntu-24.04-arm @@ -152,6 +153,19 @@ jobs: DESTDIR=${RUNNER_TEMP-${TMPDIR-/tmp}}/installed $SETARCH make test-pc "DESTDIR=$DESTDIR" + - name: CAPI extensions + uses: ./.github/actions/capiext + with: + builddir: build + if: ${{ matrix.capi_check }} + + - name: Check with previous CAPI extensions + run: | + $SETARCH make -s test-spec + env: + RUBY_TESTOPTS: '-v --tty=no' + if: ${{ matrix.capi_check }} + - uses: ./.github/actions/slack with: label: ${{ matrix.test_task }} ${{ matrix.configure }}${{ matrix.arch }} From aa173bcef826adfb84718314f4d0b4547b118730 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 11 Sep 2025 20:50:17 +0900 Subject: [PATCH 2/5] Run CAPI check separately --- .github/actions/capiext/action.yml | 10 ++++++++-- .github/workflows/macos.yml | 7 +------ .github/workflows/ubuntu.yml | 8 ++------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/.github/actions/capiext/action.yml b/.github/actions/capiext/action.yml index 4958775707a94d..e8ea87e61019d7 100644 --- a/.github/actions/capiext/action.yml +++ b/.github/actions/capiext/action.yml @@ -4,6 +4,9 @@ inputs: builddir: required: false default: '.' + make: + required: false + default: 'make -s' outputs: key: @@ -35,10 +38,13 @@ runs: path: ${{ inputs.builddir }}/spec/ruby/optional/capi/ext/ key: ${{ steps.config.outputs.key }} - - name: Force rubyspec CAPI extension binaries up to date + - name: Run test-spec with previous CAPI extension binaries shell: bash run: | - touch ${{ inputs.builddir }}/spec/ruby/optional/capi/ext/*.$DLEXT + touch spec/ruby/optional/capi/ext/*.$DLEXT + [ ! -f spec/ruby/optional/capi/ext/\*.$DLEXT ] + ${{ inputs.make }} SPECOPTS=optional/capi test-spec env: DLEXT: ${{ steps.config.outputs.DLEXT }} + working-directory: ${{ inputs.builddir }} if: ${{ steps.cache.outputs.cache-hit }} diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 9205120e2a1bbb..d22ada675545bb 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -45,7 +45,7 @@ jobs: os: macos-14 - test_task: check os: macos-15 - capi_check: true + capi_check: capi - test_task: check os: macos-13 fail-fast: false @@ -164,11 +164,6 @@ jobs: uses: ./.github/actions/capiext with: builddir: build - if: ${{ matrix.capi_check }} - - - name: Check with previous CAPI extensions - run: | - make -s test-spec env: RUBY_TESTOPTS: '-v --tty=no' if: ${{ matrix.capi_check }} diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index 2ce6a5728c04dd..5c8a072a16742c 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -39,7 +39,7 @@ jobs: - test_task: test-bundled-gems - test_task: check os: ubuntu-24.04 - capi_check: true + capi_check: capi # ubuntu-24.04-arm jobs don't start on ruby/ruby as of 2025-09-04 #- test_task: check # os: ubuntu-24.04-arm @@ -157,11 +157,7 @@ jobs: uses: ./.github/actions/capiext with: builddir: build - if: ${{ matrix.capi_check }} - - - name: Check with previous CAPI extensions - run: | - $SETARCH make -s test-spec + make: '$SETARCH make' env: RUBY_TESTOPTS: '-v --tty=no' if: ${{ matrix.capi_check }} From 17039c566b76854c8246b4a4feb2f5bd2ebf8447 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 9 Sep 2025 09:18:50 -0400 Subject: [PATCH 3/5] Don't include klass in RZombie klass is not used, so we can shrink RZombie down to 32 bytes. --- gc/default/default.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gc/default/default.c b/gc/default/default.c index aa06b7cc06c888..d5b18d20b96c8b 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -993,7 +993,7 @@ total_final_slots_count(rb_objspace_t *objspace) #endif struct RZombie { - struct RBasic basic; + VALUE flags; VALUE next; void (*dfree)(void *); void *data; @@ -2586,7 +2586,7 @@ rb_gc_impl_make_zombie(void *objspace_ptr, VALUE obj, void (*dfree)(void *), voi rb_objspace_t *objspace = objspace_ptr; struct RZombie *zombie = RZOMBIE(obj); - zombie->basic.flags = T_ZOMBIE | (zombie->basic.flags & ZOMBIE_OBJ_KEPT_FLAGS); + zombie->flags = T_ZOMBIE | (zombie->flags & ZOMBIE_OBJ_KEPT_FLAGS); zombie->dfree = dfree; zombie->data = data; VALUE prev, next = heap_pages_deferred_final; From e39fd456ac5c4c82e483431afc266b5066f00eba Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Mon, 8 Sep 2025 12:14:22 +0900 Subject: [PATCH 4/5] [ruby/prism] Use pm_arguments_end for function call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the location of CallNode was incorrect when it accepts a block parameter: ``` $ ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0]'] @ CallNode (location: (1,0)-(1,8)) # <=== It should be (1,0)-(1,9) ├── flags: ∅ ├── receiver: ∅ ├── call_operator_loc: ∅ ├── name: :foo ├── message_loc: (1,0)-(1,3) = "foo" ├── opening_loc: (1,3)-(1,4) = "(" ├── arguments: ∅ ├── closing_loc: (1,8)-(1,9) = ")" *snip* $ ruby -rprism -e 'pp Prism.parse("foo(&blk)").value.statements.body[0].slice' "foo(&blk" ``` Note that the slice lacks the closing parenthesis. https://github.com/ruby/prism/commit/3c22e6fc39 --- prism/prism.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/prism/prism.c b/prism/prism.c index afd767b84c8666..b7e2bdc1de0a13 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -18570,17 +18570,11 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b call->closing_loc = arguments.closing_loc; call->block = arguments.block; - if (arguments.block != NULL) { - call->base.location.end = arguments.block->location.end; - } else if (arguments.closing_loc.start == NULL) { - if (arguments.arguments != NULL) { - call->base.location.end = arguments.arguments->base.location.end; - } else { - call->base.location.end = call->message_loc.end; - } - } else { - call->base.location.end = arguments.closing_loc.end; + const uint8_t *end = pm_arguments_end(&arguments); + if (!end) { + end = call->message_loc.end; } + call->base.location.end = end; } } else { // Otherwise, we know the identifier is in the local table. This From f17405b69cb8b66feed289d9f857c38ce70bfe26 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Fri, 29 Aug 2025 22:35:51 +0200 Subject: [PATCH 5/5] [ruby/prism] Reject some cases with `return` and command calls The same also applies to `break`/`next`. https://bugs.ruby-lang.org/issues/21540 https://github.com/ruby/prism/commit/3a38b192e3 --- lib/prism/translation/parser/lexer.rb | 2 +- prism/prism.c | 6 ++++++ test/prism/errors/command_calls_32.txt | 19 +++++++++++++++++++ test/prism/fixtures/break.txt | 4 ++++ test/prism/fixtures/next.txt | 4 ++++ test/prism/fixtures/return.txt | 3 +++ 6 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/prism/errors/command_calls_32.txt diff --git a/lib/prism/translation/parser/lexer.rb b/lib/prism/translation/parser/lexer.rb index 222df745596428..75c48ef667c642 100644 --- a/lib/prism/translation/parser/lexer.rb +++ b/lib/prism/translation/parser/lexer.rb @@ -203,7 +203,7 @@ class Lexer # The following token types are listed as those classified as `tLPAREN`. LPAREN_CONVERSION_TOKEN_TYPES = Set.new([ :kBREAK, :tCARET, :kCASE, :tDIVIDE, :kFOR, :kIF, :kNEXT, :kRETURN, :kUNTIL, :kWHILE, :tAMPER, :tANDOP, :tBANG, :tCOMMA, :tDOT2, :tDOT3, - :tEQL, :tLPAREN, :tLPAREN2, :tLPAREN_ARG, :tLSHFT, :tNL, :tOP_ASGN, :tOROP, :tPIPE, :tSEMI, :tSTRING_DBEG, :tUMINUS, :tUPLUS + :tEQL, :tLPAREN, :tLPAREN2, :tLPAREN_ARG, :tLSHFT, :tNL, :tOP_ASGN, :tOROP, :tPIPE, :tSEMI, :tSTRING_DBEG, :tUMINUS, :tUPLUS, :tLCURLY ]) # Types of tokens that are allowed to continue a method call with comments in-between. diff --git a/prism/prism.c b/prism/prism.c index b7e2bdc1de0a13..2f7ce0b9866438 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -19102,7 +19102,13 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pm_binding_power_t binding_power = pm_binding_powers[parser->current.type].left; if (binding_power == PM_BINDING_POWER_UNSET || binding_power >= PM_BINDING_POWER_RANGE) { + pm_token_t next = parser->current; parse_arguments(parser, &arguments, false, PM_TOKEN_EOF, (uint16_t) (depth + 1)); + + // Reject `foo && return bar`. + if (!accepts_command_call && arguments.arguments != NULL) { + PM_PARSER_ERR_TOKEN_FORMAT(parser, next, PM_ERR_EXPECT_EOL_AFTER_STATEMENT, pm_token_type_human(next.type)); + } } } diff --git a/test/prism/errors/command_calls_32.txt b/test/prism/errors/command_calls_32.txt new file mode 100644 index 00000000000000..14488ca3355e25 --- /dev/null +++ b/test/prism/errors/command_calls_32.txt @@ -0,0 +1,19 @@ +foo && return bar + ^~~ unexpected local variable or method, expecting end-of-input + +tap { foo && break bar } + ^~~ unexpected local variable or method, expecting end-of-input + +tap { foo && next bar } + ^~~ unexpected local variable or method, expecting end-of-input + +foo && return() + ^ unexpected '(', expecting end-of-input + +foo && return(bar) + ^ unexpected '(', expecting end-of-input + +foo && return(bar, baz) + ^~~~~~~~~~ unexpected write target + ^ unexpected '(', expecting end-of-input + diff --git a/test/prism/fixtures/break.txt b/test/prism/fixtures/break.txt index 5532322c5cecc8..d823f866df74f5 100644 --- a/test/prism/fixtures/break.txt +++ b/test/prism/fixtures/break.txt @@ -20,6 +20,10 @@ tap { break() } tap { break(1) } +tap { (break 1) } + +tap { foo && (break 1) } + foo { break 42 } == 42 foo { |a| break } == 42 diff --git a/test/prism/fixtures/next.txt b/test/prism/fixtures/next.txt index 2ef14c6304e929..0d2d6a11f505c7 100644 --- a/test/prism/fixtures/next.txt +++ b/test/prism/fixtures/next.txt @@ -22,3 +22,7 @@ tap { next tap { next() } tap { next(1) } + +tap { (next 1) } + +tap { foo && (next 1) } diff --git a/test/prism/fixtures/return.txt b/test/prism/fixtures/return.txt index a8b5b95fabe868..952fb80da85636 100644 --- a/test/prism/fixtures/return.txt +++ b/test/prism/fixtures/return.txt @@ -22,3 +22,6 @@ return() return(1) +(return 1) + +foo && (return 1)