Skip to content

Fix clang 23 -Wlifetime-safety errors#106197

Open
azat wants to merge 1 commit into
ClickHouse:masterfrom
azat:fix/clang-23-lifetime-safety
Open

Fix clang 23 -Wlifetime-safety errors#106197
azat wants to merge 1 commit into
ClickHouse:masterfrom
azat:fix/clang-23-lifetime-safety

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented May 31, 2026

Fix:

  • -Wlifetime-safety-dangling-global-moved in ReplicasReconnector,
  • -Wlifetime-safety-dangling-field-moved in MergeTreeReaderStream and TTLTransform
  • -Wlifetime-safety-return-stack-addr-moved in BackupsWorker (move first, alias later)
  • -Wunused-but-set-variable for inside_main in keeper.
  • -Wlifetime-safety-use-after-scope-moved in ProfileEvents

Disable -Wlifetime-safety-invalidation: too many false positives, it treats mutation of a container captured by reference in a lambda as invalidating the reference to the container variable itself.

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Fix:
- -Wlifetime-safety-dangling-global-moved in `ReplicasReconnector`,
- -Wlifetime-safety-dangling-field-moved in `MergeTreeReaderStream` and `TTLTransform`
- -Wlifetime-safety-return-stack-addr-moved in `BackupsWorker` (move first, alias later)
- -Wunused-but-set-variable for `inside_main` in keeper.
- -Wlifetime-safety-use-after-scope-moved in ProfileEvents

Disable -Wlifetime-safety-invalidation: too many false positives, it treats
mutation of a container captured by reference in a lambda as invalidating the
reference to the container variable itself.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented May 31, 2026

Workflow [PR], commit [b9d2d91]

Summary:

job_name test_name status info comment
Stateless tests (arm_asan_ubsan, targeted) FAIL
Server died FAIL cidb
Scraping system tables FAIL cidb
AST fuzzer (amd_debug, targeted, old_compatibility) FAIL
Logical error: Sort order of blocks violated for column number A, left: B, right: C. Chunk D, rows read E.F (STID: 3413-350b) FAIL cidb
Stateless tests (amd_asan_ubsan, db disk, distributed plan, sequential) FAIL
03631_hive_columns_not_in_format_header ERROR cidb
Server died FAIL cidb
Segmentation fault (STID: 3973-48d2) FAIL cidb

AI Review

Summary

This PR fixes Clang 23 warning cleanup in the warning configuration and in pointer/ownership assignment order for ReplicasReconnector, MergeTreeReaderStream, TTLTransform, BackupsWorker, and ProfileEvents; keeper_main now matches the existing external inside_main pattern from the other entry points. I found no review-blocking correctness issues in the changed code, and Not for changelog is appropriate for this compiler-cleanup-only PR.

Missing context / blind spots
  • ⚠️ The available Praktika report for commit b9d2d91a currently has no test results, and GitHub still shows Build (arm_tidy) and Fast test in progress; green CI, especially the Clang 23/tidy build that exercises these warnings, would close the build-evidence gap.
Final Verdict

Status: ✅ Approve. No code changes requested; merge should still wait for the pending CI jobs that validate the compiler-warning contract.

@clickhouse-gh clickhouse-gh Bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 31, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented May 31, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.40% 84.40% +0.00%
Functions 91.40% 91.40% +0.00%
Branches 77.00% 77.00% +0.00%

Changed lines: Changed C/C++ lines covered by tests: 35/38 (92.11%) | Lost baseline coverage (was covered on master, now uncovered in this PR): 2 line(s) · Uncovered code

Full report · Diff report

@alexey-milovidov
Copy link
Copy Markdown
Member

it treats mutation of a container captured by reference in a lambda as invalidating the reference to the container variable itself

We can annotate these cases in a subsequent PR and enable the warnings. I don't mind doing it in 1000 places :)

@azat
Copy link
Copy Markdown
Member Author

azat commented Jun 1, 2026

We can annotate these cases in a subsequent PR and enable the warnings. I don't mind doing it in 1000 places :)

There are not too many (AFAFIR around 63), but not sure that it worth it, since looks like it is a bug in the clang analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants