fix(tools): collapse intra-file duplicate typedefs in base header#2149
Open
lloeki wants to merge 1 commit into
Open
fix(tools): collapse intra-file duplicate typedefs in base header#2149lloeki wants to merge 1 commit into
lloeki wants to merge 1 commit into
Conversation
Contributor
📚 Documentation Check Results📦
|
Contributor
Clippy Allow Annotation ReportComparing clippy allow annotations between branches:
Summary by Rule
Annotation Counts by File
Annotation Stats by Crate
About This ReportThis report tracks Clippy allow annotations for specific rules, showing how they've changed in this PR. Decreasing the number of these annotations generally improves code quality. |
|
Contributor
🔒 Cargo Deny Results📦
|
This was referenced Jun 22, 2026
dedup_headers only removed definitions from child headers that were
byte-identical to ones already present in the base header. It never
deduplicated definitions within the base header itself, so cbindgen
output that emits the same profiling type from two crate boundaries
left duplicate typedefs in the merged common.h.
Two cases survived and broke consumers compiling with
-Werror -Wtypedef-redefinition (C11):
1. A bare forward declaration "typedef struct X X;" coexisting with the
full-body definition "typedef struct X { ... } X;" (e.g.
ddog_prof_EncodedProfile, ddog_prof_StringId, OpaqueStringId).
2. An identical pointer typedef emitted twice whose doc comments differ,
so the existing exact-string dedup kept both (ddog_prof_StringId2,
ddog_prof_MappingId2, ddog_prof_FunctionId2).
Add a final pass over the assembled base header that drops a forward
struct/union/enum declaration when a full-body definition of the same
name exists, and removes later duplicates of an identical typedef
statement regardless of differing comments. This makes the generated
common.h clean by construction and removes the need for downstream
post-processing workarounds.
lloeki
force-pushed
the
lloeki/fix-dedup-headers-intra-file-typedefs
branch
from
ab502c6 to
77f60a7
Compare
yannham
reviewed
Jun 22, 2026
Comment on lines
+123
to
+125
| match s.find("*/") { | ||
| Some(end) => s[end + 2..].trim_start(), | ||
| None => s, |
Contributor
There was a problem hiding this comment.
Suggested change
| match s.find("*/") { | |
| Some(end) => s[end + 2..].trim_start(), | |
| None => s, | |
| s.find("*/").map(|end| s[end + 2..].trim_start()).unwrap_or(s); |
| } | ||
|
|
||
| fn is_ident(s: &str) -> bool { | ||
| !s.is_empty() && s.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'_') |
Contributor
There was a problem hiding this comment.
For a token to be a valid C identifier, the first character must NOT be a digit, which isn't included in this test. I don't remember in C but in Rust for example this could match 0usize which isn't an identifier.
| .filter_map(|d| bodied_typedef_name(statement_text(d.str))) | ||
| .collect(); | ||
|
|
||
| let mut seen: HashSet<&str> = HashSet::new(); |
Contributor
There was a problem hiding this comment.
It's not entirely clear if what seen includes going forward. I would suggest to give it a more explicit name (I suppose it's the typedefs that are not bodied?)
Contributor
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Enhance the
dedup_headersdev tool so the generated/bundledinclude/datadog/common.hno longer contains duplicate typedefs.Why
dedup_headersonly removed definitions from child headers that werebyte-identical to ones already present in the base header (
common.h).It never deduplicated definitions within the base header. When cbindgen
emits the same profiling type from two crate boundaries (e.g. via
after_includesforward declarations inlibdd-profiling-ffi/cbindgen.tomlplus the regular body definition), the merged
common.hends up withduplicate typedefs.
These are fatal for consumers compiling with
-Werror -Wtypedef-redefinition(C11). Two distinct classes were observed in the v36.0.0 artifacts:
typedef struct X X;forwarddeclaration coexisting with the full
typedef struct X { ... } X;:ddog_prof_EncodedProfile,ddog_prof_StringId,OpaqueStringId.one carries a doc comment (so the existing exact-string dedup keeps both):
ddog_prof_StringId2,ddog_prof_MappingId2,ddog_prof_FunctionId2.How
Add a final pass (
dedup_base_typedefs) over the assembled base header that:typedef struct/union/enum X X;when a full-bodydefinition of the same name
Xexists elsewhere in the file (keeping thebody, regardless of ordering), and
statement text with any leading doc comment stripped.
Opaque forward declarations (no body) and genuine aliases
(
typedef struct A B;withA != B) are preserved.This makes
common.hclean by construction and obsoletes downstreampost-processing workarounds (e.g. the one in
libdatadog-rb).Validation
Headers were generated via the FFI crates' cbindgen build scripts and run
through
dedup_headersexactly asbuilderinvokes it.Before — each of the six types appears twice; clang fails:
After — each type appears exactly once; clang passes (exit 0):
cargo test -p tools --lib— 20 passed (5 new tests for the dedup pass)cargo clippy -p tools --all-targets --all-features -- -D warnings— cleancargo fmt -p tools -- --check— cleanNote
One of three coordinated changes for the libdatadog v36 duplicate-typedef header issue (increasing order of permanence):
-Wno-error=typedef-redefinitionstopgap.36.0.0.1.1, without waiting for a libdatadog release.dedup_headers: makescommon.hclean by construction and obsoletes the libdatadog-rb post-processing once released.