Skip to content

Commit 4d52345

Browse files
committed
Lint the windows too: tools/lint_clang.ps1 checks src/ui and tests/ui
- The clang lint left src/ui and tests/ui out for want of Slint's headers, and 1.3.3 was lost to what that hid: a NUL byte inside a string literal in tests/ui, which MSVC accepts and GCC refuses. It takes Slint's headers and the generated components from the local preset's tree (build\windows-msvc) now, says to build that when it is missing, and checks everything CI compiles with -Werror: 197 files, about 15 minutes. Run against the 1.3.3 test file, it reports the NUL at the line GCC stopped on. - Its first runs over the windows found five things only clang reports, all fixed: two GetProcAddress results cast to a function type they do not match (now through `void (*)()`), a volatile parameter (deprecated since C++20) in the stack-overflow crash helper, and two floats widened to double without saying so. The crash tests still see the stack overflow.
1 parent 87ca73f commit 4d52345

5 files changed

Lines changed: 30 additions & 15 deletions

File tree

‎src/ui/crash_report.cpp‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,10 @@ void CrashReport::install(const std::filesystem::path& directory, std::string_vi
262262
// Looked up now, while the process is healthy: loading a DLL from inside a crash is how a
263263
// crash handler becomes a hang.
264264
if (const HMODULE dbghelp = LoadLibraryW(L"dbghelp.dll")) {
265-
g_writeDump =
266-
reinterpret_cast<MiniDumpWriteDumpFn>(GetProcAddress(dbghelp, "MiniDumpWriteDump"));
265+
// Through `void (*)()`, the one function type any other is cast to and from without a
266+
// compiler saying the two do not match: GetProcAddress hands back one that does not.
267+
g_writeDump = reinterpret_cast<MiniDumpWriteDumpFn>(
268+
reinterpret_cast<void (*)()>(GetProcAddress(dbghelp, "MiniDumpWriteDump")));
267269
}
268270

269271
redirectStderr();
@@ -362,10 +364,11 @@ namespace {
362364

363365
/// Deeper until the stack runs out. Each frame writes an array of its own and adds to what the
364366
/// next returns, so the optimiser can neither drop the frames nor turn the calls into a loop.
365-
int deeper(volatile int depth) {
367+
int deeper(int depth) {
368+
volatile int here = depth; // a volatile parameter is deprecated since C++20; a local is not
366369
volatile char frame[1024];
367-
frame[0] = static_cast<char>(depth);
368-
return depth < 0 ? frame[0] : deeper(depth + 1) + frame[0];
370+
frame[0] = static_cast<char>(here);
371+
return here < 0 ? frame[0] : deeper(here + 1) + frame[0];
369372
}
370373

371374
} // namespace

‎src/ui/native_window.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,9 @@ LogicalExtent fitToScreen(LogicalExtent wanted) noexcept {
274274
UINT dpi = 0;
275275
if (const HMODULE shcore = LoadLibraryW(L"shcore.dll"); shcore != nullptr) {
276276
using GetDpiForMonitorFn = HRESULT(WINAPI*)(HMONITOR, int, UINT*, UINT*);
277-
const auto getDpi =
278-
reinterpret_cast<GetDpiForMonitorFn>(GetProcAddress(shcore, "GetDpiForMonitor"));
277+
// Through `void (*)()`, as `CrashReport::install` casts what GetProcAddress returns.
278+
const auto getDpi = reinterpret_cast<GetDpiForMonitorFn>(
279+
reinterpret_cast<void (*)()>(GetProcAddress(shcore, "GetDpiForMonitor")));
279280
UINT x = 0;
280281
UINT y = 0;
281282
constexpr int kEffectiveDpi = 0; // MDT_EFFECTIVE_DPI

‎src/ui/rule_text.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ trigger::Value parseValue(std::string_view text) {
8888
return trigger::Value::ofInt(static_cast<std::int32_t>(*number));
8989
}
9090
// Clamped to what a float holds, which converting past is undefined behaviour (L5).
91-
const double most = std::numeric_limits<float>::max();
91+
const double most = static_cast<double>(std::numeric_limits<float>::max());
9292
return trigger::Value::ofFloat(static_cast<float>(std::clamp(*number, -most, most)));
9393
}
9494
return trigger::Value::ofText(trimmed);

‎src/ui/rules_controller.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ RulesController::RulesController(output::OutputRunner& runner,
217217
window_->on_rule_muted_changed(finishing([this](bool on) { setMuted(on); }));
218218
window_->on_rule_muted_at(finishing([this](int index, bool on) { setMutedAt(index, on); }));
219219
window_->on_fold_clicked(finishing([this](int which) { toggleFold(which); }));
220-
window_->on_rule_rate_changed(finishing([this](float factor) { nudgeRate(factor); }));
220+
window_->on_rule_rate_changed(
221+
finishing([this](float factor) { nudgeRate(static_cast<double>(factor)); }));
221222
// The name commits on every keystroke, so there is never anything of its own to carry.
222223
window_->on_rule_renamed([this](const slint::SharedString& n) { rename(std::string(n)); });
223224
window_->on_rule_tested(finishing([this] { test(); }));

‎tools/lint_clang.ps1‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@
1717
# powershell -ExecutionPolicy Bypass -File tools/lint_clang.ps1
1818
# powershell -ExecutionPolicy Bypass -File tools/lint_clang.ps1 src/core/control
1919
#
20-
# `src/core`, `src/cli` and `tests` by default — the engine, the console and their tests, which
21-
# CI compiles with -Werror. `src/ui` and `tests/ui` are left out, by name: they need Slint's
22-
# headers and the generated `main_window.h`, which this script does not reconstruct. full.yml
23-
# builds them on every push, with GCC as well since the linux-tsan job.
20+
# `src/core`, `src/cli`, `src/ui` and `tests` by default — everything CI compiles with -Werror.
21+
# **The windows too**: `src/ui` and `tests/ui` need Slint's headers and the generated
22+
# `main_window.h`, and take them from the `local` preset's tree (`build\windows-msvc`), so build
23+
# that first. They were left out until 2026-10-08, and 1.3.3 was lost to what that hid: a NUL
24+
# byte inside a string literal in tests/ui, which MSVC accepts and GCC refuses.
2425
#
2526
# **A file clang could not parse is not a clean file.** It stops at the first header it cannot
2627
# find and then says nothing — which this script used to count as clean, for four tests/ui
@@ -30,8 +31,8 @@
3031
# Exits 1 if clang reports anything, 3 if a file could not be checked, 0 only if every file
3132
# was checked and nothing was found.
3233

33-
param([string[]]$Paths = @('src/core', 'src/cli', 'tests'))
34-
$Exclude = @('src\ui\', 'tests\ui\')
34+
param([string[]]$Paths = @('src/core', 'src/cli', 'src/ui', 'tests'))
35+
$Exclude = @()
3536

3637
# NOT 'Stop': Windows PowerShell wraps a native command's stderr in ErrorRecords, and
3738
# clang-tidy writes its "N warnings generated" summary there — which under 'Stop' aborts
@@ -48,6 +49,9 @@ if (-not (Test-Path "$build\generated")) {
4849
Write-Error "configure the local-core preset first: no $build\generated"
4950
exit 2
5051
}
52+
# The windows' Slint headers and generated components, from the `local` preset's tree.
53+
$app = Join-Path $repo 'build\windows-msvc'
54+
$appBuilt = Test-Path "$app\src\ui\main_window.h"
5155

5256
# The include roots and defines takt4_tests is compiled with, from src/CMakeLists.txt and
5357
# tests/CMakeLists.txt. Kept by hand; if a target grows a dependency, add it here.
@@ -67,6 +71,9 @@ $flags = @(
6771
"-isystem$build\_deps\pugixml-src\src",
6872
"-isystem$build\_deps\miniz-src",
6973
'-DPUGIXML_NO_XPATH', '-DMINIZ_NO_STDIO', '-DMINIZ_NO_TIME', '-DMINIZ_NO_ZLIB_COMPATIBLE_NAMES',
74+
"-I$app\src\ui",
75+
"-isystem$app\_deps\slint-src\api\cpp\include",
76+
"-isystem$app\_deps\slint-build\generated_include",
7077
"-isystem$repo\third_party\portaudio\include",
7178
"-isystem$repo\third_party\link\include",
7279
"-isystem$repo\third_party\link\modules\asio-standalone\asio\include",
@@ -150,6 +157,9 @@ if ($skipped.Count -gt 0) {
150157
if ($unchecked.Count -gt 0) {
151158
Write-Output "`n$($unchecked.Count) file(s) not checked - a header clang could not find, which is this script's include list, not the code:"
152159
$unchecked | ForEach-Object { Write-Output " $_" }
160+
if (-not $appBuilt) {
161+
Write-Output " (src/ui and tests/ui take Slint's headers from build\windows-msvc: build the local preset)"
162+
}
153163
}
154164
if ($found -gt 0) { Write-Output "`n$found file(s) with findings"; exit 1 }
155165
if ($unchecked.Count -gt 0) { Write-Output "`nnothing found in the files that were checked, but not every file was"; exit 3 }

0 commit comments

Comments
 (0)