build: workflows: Add a rules for checking incompatible types strictly#10655
build: workflows: Add a rules for checking incompatible types strictly#10655patrick-stephens merged 2 commits intomasterfrom
Conversation
cosmo0920
requested review from
celalettin1286,
fujimotos,
niedbalski and
patrick-stephens
as code owners
patrick-stephens
left a comment
patrick-stephens
left a comment
There was a problem hiding this comment.
We should update to Ubuntu 24 soon really as well, it is the latest one so 22 will go.
|
I recommend renaming the new option for clarity to |
Sounds good. I'll rename it later. |
cosmo0920
force-pushed
the
cosmo0920-deny-incompatible-types
branch
from
9d7cc2d to
2e94779
Compare
cosmo0920
force-pushed
the
cosmo0920-deny-incompatible-types
branch
from
0d7a9ea to
a709772
Compare
WalkthroughAdds a CMake option to enable -Werror=incompatible-pointer-types and updates the GitHub Actions unit-test matrix to add a dedicated clang job that runs with this option while excluding that pairing from the default clang runs. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Matrix as Matrix Expander
participant Job as Unit Tests Job
participant CMake as CMake Configure
participant Test as Test Runner
Dev->>GH: push PR / update workflow
GH->>Matrix: expand matrix
rect rgba(220,235,240,0.4)
note right of Matrix: matrix now includes\n`-DFLB_COMPILER_STRICT_POINTER_TYPES=On`
Matrix->>Job: schedule jobs (with excludes)
Job->>Job: ensure default clang excludes strict combo
Job->>Job: include dedicated clang+strict job
end
Job->>CMake: configure with `-DFLB_COMPILER_STRICT_POINTER_TYPES=On` (clang job)
CMake-->>Job: apply strict pointer warning flags
Job->>Test: build & run unit tests
Test-->>GH: post results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
🔇 Additional comments (1)
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. Comment |
cosmo0920
changed the title
cosmo0920
changed the title
cosmo0920
force-pushed
the
cosmo0920-deny-incompatible-types
branch
from
a709772 to
12b7529
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/unit-tests.yaml (2)
56-56: Add macOS coverage for strict pointer types.You’re only exercising the flag on Ubuntu. Given the AppleClang-specific handling in CMake, add a macOS matrix entry to catch AppleClang-only issues.
Apply:
matrix: flb_option: - "-DFLB_JEMALLOC=Off" - "-DFLB_SANITIZE_MEMORY=On" - "-DFLB_SANITIZE_THREAD=On" + - "-DFLB_COMPILER_STRICT_POINTER_TYPES=On"
75-78: Clang path for strict-pointer-types is excluded; add an explicit include to still run it.Right now clang + strict-pointer-types never runs in this job. If that’s intentional for the cross product, explicitly re-add it via include to ensure we test clang with this flag.
Apply:
matrix: flb_option: - "-DFLB_JEMALLOC=On" ... - "-DFLB_COMPILER_STRICT_POINTER_TYPES=On" cmake_version: - "3.31.6" compiler: - gcc: cc: gcc cxx: g++ - clang: cc: clang cxx: clang++ + include: + - flb_option: "-DFLB_COMPILER_STRICT_POINTER_TYPES=On" + compiler: + cc: clang + cxx: clang++ exclude: - flb_option: "-DFLB_COVERAGE=On" compiler: cc: clang cxx: clang++ - flb_option: "-DFLB_ARROW=On" compiler: cc: clang cxx: clang++ - flb_option: "-DFLB_COMPILER_STRICT_POINTER_TYPES=On" compiler: cc: clang cxx: clang++CMakeLists.txt (1)
450-459: Scope flags to C-only to avoid leaking unknown-warning options into C++.add_compile_options applies to all languages; use generator expressions to constrain to C to prevent noisy “unknown warning option” from C++ compilations.
Apply:
-if(FLB_COMPILER_STRICT_POINTER_TYPES) - if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang|AppleClang") - add_compile_options(-Werror=incompatible-pointer-types) - endif() - # Currently, AppleClang has more struct rules when using -Werror=incompatible-pointer-types. - # We still permit to discarding const qualifiers warnings. - if(CMAKE_C_COMPILER_ID MATCHES "AppleClang") - add_compile_options(-Wno-incompatible-pointer-types-discards-qualifiers) - endif() -endif() +if(FLB_COMPILER_STRICT_POINTER_TYPES) + if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang|AppleClang") + add_compile_options($<$<COMPILE_LANGUAGE:C>:-Werror=incompatible-pointer-types>) + endif() + # AppleClang: allow discarding const-qualifier diagnostics without failing the build. + if(CMAKE_C_COMPILER_ID MATCHES "AppleClang") + add_compile_options($<$<COMPILE_LANGUAGE:C>:-Wno-incompatible-pointer-types-discards-qualifiers>) + # Alternative (keeps warning visible): -Wno-error=incompatible-pointer-types-discards-qualifiers + endif() +endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/unit-tests.yaml(2 hunks)CMakeLists.txt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-31T12:46:11.940Z
Learnt from: ThomasDevoogdt
PR: fluent/fluent-bit#9277
File: .github/workflows/pr-compile-check.yaml:147-151
Timestamp: 2025-08-31T12:46:11.940Z
Learning: In fluent-bit CMakeLists.txt, the system library preference flags are defined as FLB_PREFER_SYSTEM_LIB_ZSTD and FLB_PREFER_SYSTEM_LIB_KAFKA with the FLB_ prefix.
Applied to files:
CMakeLists.txt.github/workflows/unit-tests.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (1)
CMakeLists.txt (1)
161-161: Option name LGTM and consistent with FLB_ flags.*Clear name and default. No issues.
cosmo0920
force-pushed
the
cosmo0920-deny-incompatible-types
branch
from
12b7529 to
c458595
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/unit-tests.yaml (1)
75-78: Approve exclusion pattern; add clarifying comment.The exclusion syntax is correct and consistent with existing patterns (e.g., lines 67–74). However, consider adding an inline comment to explain why clang is excluded from this option—e.g., whether clang support is deferred, intentionally testing only GCC, or another reason.
Also note: The AI-generated summary mentions "a clang-specific matrix entry to run clang with this option," but the provided workflow shows only the exclusion. Verify that the clang-specific job exists elsewhere in the workflow or is part of the planned implementation.
Example clarification:
- - flb_option: "-DFLB_COMPILER_STRICT_POINTER_TYPES=On" + # Test strict pointer types with GCC only (clang support pending) + - flb_option: "-DFLB_COMPILER_STRICT_POINTER_TYPES=On" compiler: cc: clang cxx: clang++
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/unit-tests.yaml(2 hunks)CMakeLists.txt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-without-cxx (3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COMPILER_STRICT_POINTER_TYPES=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_ARROW=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
🔇 Additional comments (1)
.github/workflows/unit-tests.yaml (1)
56-56: Option added to matrix correctly.The new
-DFLB_COMPILER_STRICT_POINTER_TYPES=Onoption is properly formatted and positioned in the test matrix. This will expand the matrix to test the strict pointer-type checking with GCC.Verify that the corresponding CMakeLists.txt properly defines the
FLB_COMPILER_STRICT_POINTER_TYPESoption and applies the intended compiler flags (-Werror=incompatible-pointer-typesfor GCC/Clang, with appropriate qualifiers handling).
|
thanks, lets put this in 2 commits only |
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
cosmo0920
force-pushed
the
cosmo0920-deny-incompatible-types
branch
from
c458595 to
9bd8e76
Compare
3 participants
This is because this behavior is default since gcc 14 or later.
But our CI is still using ubuntu-22.04 as a base testing workers.
So, we need to implement an additional task for denying incompatible
types for pointer operations.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Tests
Chores