Skip to content

fix: support header_contents() with clang_macro_fallback#3353

Open
zRedShift wants to merge 1 commit into
rust-lang:mainfrom
zRedShift:fix-macro-fallback-header-contents-target
Open

fix: support header_contents() with clang_macro_fallback#3353
zRedShift wants to merge 1 commit into
rust-lang:mainfrom
zRedShift:fix-macro-fallback-header-contents-target

Conversation

@zRedShift
Copy link
Copy Markdown

@zRedShift zRedShift commented Mar 14, 2026

Summary

Fixes #3351.

clang_macro_fallback() builds a fallback translation unit from the input headers. When the user only provides input through header_contents(), input_headers is empty, so fallback setup returns None and macros that need fallback evaluation are silently skipped.

Keep input_header_contents available to the generation context, materialize those virtual headers as temporary fallback headers while the PCH is alive, and include them in the fallback PCH setup.

The implicit --target fix from the original version of this PR was split out to #3382.

@zRedShift zRedShift force-pushed the fix-macro-fallback-header-contents-target branch 5 times, most recently from a499369 to e9fcc7c Compare March 14, 2026 22:27
Copy link
Copy Markdown
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bunch of other drive-by changes like where we put the `.macro_eval

View changes since this review

Comment thread bindgen/ir/context.rs Outdated
.iter()
.filter_map(|(_, contents, original_name)| {
// Sanitize the user-provided name so it always
// resolves under build_dir:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused, why is this needed? Is it intended to be some sort of path traversal escape prevention thing?

I'd rather avoid this, I don't think there's any use case for relative directories in header_contents(), tbh. In any case, this is just eating the FS errors etc, which seems bad. Let's either propagate it or crash?

@emilio
Copy link
Copy Markdown
Contributor

emilio commented May 4, 2026

I'd split the --target fix to a separate PR if possible.

@jgabaut
Copy link
Copy Markdown

jgabaut commented Jun 5, 2026

Any news on this? I opened #3381 but I think I just had to update my dependency's binding generation to have explicit target arguments for mingw-w64.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 6, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@zRedShift zRedShift changed the title fix: clang_macro_fallback with header_contents() and implicit --target fix: support header_contents() with clang_macro_fallback Jun 6, 2026
@zRedShift
Copy link
Copy Markdown
Author

Thanks, I split this up and rewrote this branch to be #3351-only.

The implicit --target fix is now #3382. For this PR, I removed the original path-preservation / path traversal handling and the broader tests around that behavior. The fallback now materializes generated scratch headers under the fallback build dir and panics on materialization write errors instead of silently swallowing them.

@zRedShift zRedShift force-pushed the fix-macro-fallback-header-contents-target branch from 50cf047 to 36e96d6 Compare June 6, 2026 11:55
@zRedShift zRedShift force-pushed the fix-macro-fallback-header-contents-target branch from 36e96d6 to 6e57d30 Compare June 6, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clang_macro_fallback() silently skipped when using header_contents()

4 participants