Skip to content

throw.h: use setjmp/longjmp with clang on MinGW - #7772

Merged
matz merged 1 commit into
mruby:masterfrom
b84330808:mingw-clang-setjmp
Nov 8, 2026
Merged

matz merged 1 commit into
mruby:masterfrom
b84330808:mingw-clang-setjmp

Conversation

@b84330808

@b84330808 b84330808 commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

The MinGW branch picks __builtin_setjmp/__builtin_longjmp for any
compiler that defines GNUC >= 4, and clang does. Clang declares
__builtin_setjmp as taking a void **, while jmp_buf on mingw-w64 is an
array of struct _SETJMP_FLOAT128, so llvm-mingw (clang 23) stops at the
first MRB_TRY/MRC_TRY with -Wincompatible-pointer-types, which is an
error there:

codegen.c:221:3: error: incompatible pointer types passing
'jmp_buf' (aka 'struct _SETJMP_FLOAT128[16]') to parameter of type
'void **'

Casting the buffer to void ** gets the build through, but then the
first exception crashes: begin; raise "x"; rescue; end segfaults and
mrbtest dies before its first test, as #5950 reported. Leave clang out
of the builtin branch, as was done for ARM64 in #6637, so it uses the
setjmp()/longjmp() from <setjmp.h>. GCC keeps the builtins. The same
condition is in mrc_throw.h.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compatibility when building with Clang on MinGW64. These builds now use the standard jump-handling fallback, while eligible non-Clang builds continue to use the existing built-in approach. This prevents Clang builds from selecting an incompatible compiler-specific path and helps ensure consistent compilation across supported toolchains.

The MinGW branch picks __builtin_setjmp/__builtin_longjmp for any
compiler that defines __GNUC__ >= 4, and clang does. Clang declares
__builtin_setjmp as taking a void **, while jmp_buf on mingw-w64 is an
array of struct _SETJMP_FLOAT128, so llvm-mingw (clang 23) stops at the
first MRB_TRY/MRC_TRY with -Wincompatible-pointer-types, which is an
error there:

    codegen.c:221:3: error: incompatible pointer types passing
    'jmp_buf' (aka 'struct _SETJMP_FLOAT128[16]') to parameter of type
    'void **'

Casting the buffer to void ** gets the build through, but then the
first exception crashes: `begin; raise "x"; rescue; end` segfaults and
mrbtest dies before its first test, as mruby#5950 reported. Leave clang out
of the builtin branch, as was done for ARM64 in mruby#6637, so it uses the
setjmp()/longjmp() from <setjmp.h>. GCC keeps the builtins. The same
condition is in mrc_throw.h.
@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Both throw headers now exclude Clang from the MinGW64 built-in jump-function path. Clang builds use the existing setjmp/longjmp fallback.

Changes: Clang jump-function fallback

Layer / File(s) Summary
Jump-function selection
include/mruby/throw.h, mrbgems/mruby-compiler/include/mrc_throw.h
The MinGW64 built-in jump-function condition now excludes Clang. Clang builds use the existing setjmp/longjmp fallback.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Suggested labels: core, mrbgems

Merge Risk: Minimal · up to 9aaf1

Clang builds use the standard jump functions. No actionable merge-blocking risk is established.

Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "throw.h: use setjmp/longjmp with clang on MinGW" matches the main change. The PR stops the MinGW64 __builtin_setjmp/__builtin_longjmp path in throw.h and mrc_throw.h when compiling with Cla…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title "throw.h: use setjmp/longjmp with clang on MinGW" matches the main change. The PR stops the MinGW64 __builtin_setjmp/__builtin_longjmp path in throw.h and mrc_throw.h when compiling with Clang, so Clang builds on MinGW use the standard setjmp/longjmp instead. GCC keeps the builtins. The title names the file, the functions, the compiler, and the platform, so a reader scanning history can identify the change.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 2 files.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c6e44704-92fc-450f-aa6b-2893e8613bf6
Commits

Reviewing files that changed from the base of the PR and between 3fd8110 and 9aaf1a2.

Files selected for processing: 2

  • include/mruby/throw.h
  • mrbgems/mruby-compiler/include/mrc_throw.h

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

@matz
matz merged commit de714ea into mruby:master Nov 8, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants