test: the Wayland libdecor fixture builds with Clang 23 (-Wunused-but-set-global) #2290

Closed
mooki000 wants to merge 1 commit from test/wayland-libdecor-clang23 into main
mooki000 commented 2026-11-11 13:43:09 +00:00 (Migrated from github.com)

tests/test_wayland_libdecor_hint.py fails before it runs a case with Clang 23 (the readme: "LLVM 23 or newer reads
the play profile"; tests/run_all.py takes clang++ before g++): its C++ fixture is built with -Werror, and
Clang 23's -Wall now includes -Wunused-but-set-global.

Part of #2334.

harness.cpp:25:12: error: variable 'sdl_ready' set but not used [-Werror,-Wunused-but-set-global]

What changed

  • The fixture declares the sdl_ready global that the function under test (hos_display_init from
    host/posix/display_sdl.c) sets, but never read it. The warning is right about the fixture: in the real file
    hos_display_owns_sdl reads it. The harness now prints it beside the init result (sdl2: init 1, ready 1,
    headless: init 0, ready 0). No warning is switched off.

How it was tested

Linux (Arch):

  • Clang 23.1.3: fails on main with the error above; passes with the change (3 cases, 0 failed).
  • GCC 16.2.1: passes before and after.
  • -Wunused-but-set-global comes with -Wall in Clang 23 (a three-line file whose global is only written: warned with
    -Wall, not with -Wextra alone).
  • python -I -B tests/run_all.py on main 49f75dc8 with this change (2026-11-11, Clang 23 as clang++, load about 70
    to 140): 362 passed, 2 failed, 10 skipped; tests/test_wayland_libdecor_hint.py passes. tests/test_updater_asset_digest.py
    fails on main as well on this machine (only a .NET 10 SDK; a pull request of its own). tests/test_bake_skeleton.py
    failed when one of its compiler processes ended with no output under that load; run alone afterwards it passes (9
    tests, 110 s).

Not tested

  • CI's Clang (Ubuntu 24.04's) was not run here; the change only adds a read, which no compiler warns about.
  • The check is skipped on Windows (RUN_ALL_SKIP), as before.

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 [email protected]

`tests/test_wayland_libdecor_hint.py` fails before it runs a case with Clang 23 (the readme: "LLVM 23 or newer reads the play profile"; `tests/run_all.py` takes `clang++` before `g++`): its C++ fixture is built with `-Werror`, and Clang 23's `-Wall` now includes `-Wunused-but-set-global`. Part of #2334. ``` harness.cpp:25:12: error: variable 'sdl_ready' set but not used [-Werror,-Wunused-but-set-global] ``` ## What changed - The fixture declares the `sdl_ready` global that the function under test (`hos_display_init` from `host/posix/display_sdl.c`) sets, but never read it. The warning is right about the fixture: in the real file `hos_display_owns_sdl` reads it. The harness now prints it beside the init result (`sdl2: init 1, ready 1`, `headless: init 0, ready 0`). No warning is switched off. ## How it was tested Linux (Arch): - Clang 23.1.3: fails on main with the error above; passes with the change (3 cases, 0 failed). - GCC 16.2.1: passes before and after. - `-Wunused-but-set-global` comes with `-Wall` in Clang 23 (a three-line file whose global is only written: warned with `-Wall`, not with `-Wextra` alone). - `python -I -B tests/run_all.py` on main 49f75dc8 with this change (2026-11-11, Clang 23 as `clang++`, load about 70 to 140): 362 passed, 2 failed, 10 skipped; `tests/test_wayland_libdecor_hint.py` passes. `tests/test_updater_asset_digest.py` fails on main as well on this machine (only a .NET 10 SDK; a pull request of its own). `tests/test_bake_skeleton.py` failed when one of its compiler processes ended with no output under that load; run alone afterwards it passes (9 tests, 110 s). ## Not tested - CI's Clang (Ubuntu 24.04's) was not run here; the change only adds a read, which no compiler warns about. - The check is skipped on Windows (`RUN_ALL_SKIP`), as before. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>
mooki000 commented 2026-11-11 17:08:08 +00:00 (Migrated from github.com)

Rebased onto main b29967bf so the workflows run here: they had waited for an approval on the old head, and main's game-free tests are green again since #2380. The patch is unchanged (git range-diff: the commit is equal), and its test passes on the new base.

Rebased onto main `b29967bf` so the workflows run here: they had waited for an approval on the old head, and main's game-free tests are green again since #2380. The patch is unchanged (`git range-diff`: the commit is equal), and its test passes on the new base.
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 20:07:45 +00:00
Joey-Magnus (Migrated from github.com) left a comment

Thanks for tracking this down with a real Clang 23 run. The diagnosis was right, but main already fixed the same warning a different way, so this PR is no longer needed.

question tests/test_wayland_libdecor_hint.py:42: #2171 (b5d67932, merged 2026-11-11 11:57) added int hos_display_owns_sdl(void) { return sdl_ready; } to the harness. Its commit message says "a one-line display-fixture declaration fixes an upstream Clang 23 unused-state warning". The global now has a reader, so -Wunused-but-set-global no longer fires on main. Your test run was on 49f75dc8, which predates #2171. The rebase onto b29967bf picked #2171 up without a conflict, which is why the diff still looks valid. On the current head, the extra sdl_ready in the printf (line 51) is a second read of a global that is already used, and the comment beside it gives a reason that no longer applies. Could you confirm with your Clang 23 that main passes without this change? If it does, I'd close this as superseded by #2171 (and maybe note that on #2334).

The two red CI jobs have nothing to do with this change. Windows part 1/2 fails in tests/test_native_astc_texture_count.py, and the appimage job timed out on a network download while installing system packages.

Thanks for tracking this down with a real Clang 23 run. The diagnosis was right, but main already fixed the same warning a different way, so this PR is no longer needed. **question** `tests/test_wayland_libdecor_hint.py:42`: #2171 (b5d67932, merged 2026-11-11 11:57) added `int hos_display_owns_sdl(void) { return sdl_ready; }` to the harness. Its commit message says "a one-line display-fixture declaration fixes an upstream Clang 23 unused-state warning". The global now has a reader, so `-Wunused-but-set-global` no longer fires on main. Your test run was on 49f75dc8, which predates #2171. The rebase onto b29967bf picked #2171 up without a conflict, which is why the diff still looks valid. On the current head, the extra `sdl_ready` in the `printf` (line 51) is a second read of a global that is already used, and the comment beside it gives a reason that no longer applies. Could you confirm with your Clang 23 that main passes without this change? If it does, I'd close this as superseded by #2171 (and maybe note that on #2334). The two red CI jobs have nothing to do with this change. Windows part 1/2 fails in `tests/test_native_astc_texture_count.py`, and the appimage job timed out on a network download while installing system packages.
Joey-Magnus (Migrated from github.com) reviewed 2026-11-11 21:24:33 +00:00
Joey-Magnus (Migrated from github.com) left a comment

I think #2171 already covers this: main's tests/test_wayland_libdecor_hint.py:42 has int hos_display_owns_sdl(void) { return sdl_ready; }, so the global already has a reader. This would add a redundant one, so it can probably be closed.

I think #2171 already covers this: main's `tests/test_wayland_libdecor_hint.py:42` has `int hos_display_owns_sdl(void) { return sdl_ready; }`, so the global already has a reader. This would add a redundant one, so it can probably be closed.
mooki000 commented 2026-11-11 22:03:26 +00:00 (Migrated from github.com)

Confirmed on clean main 940eb9ef with Clang 23.1.3: wayland libdecor hint: 3 cases, 0 failed (SDL2, SDL3 and headless). #2171’s hos_display_owns_sdl() already reads the state, so this PR’s extra printf read is redundant. Closing as superseded; thanks for catching it.

Confirmed on clean main `940eb9ef` with Clang 23.1.3: `wayland libdecor hint: 3 cases, 0 failed` (SDL2, SDL3 and headless). #2171’s `hos_display_owns_sdl()` already reads the state, so this PR’s extra printf read is redundant. Closing as superseded; thanks for catching it.
Commenting is not possible because the repository is archived.
No description provided.