Conversation
`AnnotationSet::parse_atom` unwrapped the result of `FromStr`, so an annotation such as `cbindgen:rename-all=not-a-valid-rename-rule` made cbindgen panic. Log a warning and ignore the annotation instead, so the configured default is used. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
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.
An annotation with a value that doesn't parse, such as
made cbindgen panic with
called `Option::unwrap()` on a `None` value, becauseAnnotationSet::parse_atomdidy.parse::<T>().ok().unwrap().With this change
parse_atomlogs a warning with the existingwarn!macro and returnsNone, so the annotation is ignored and the callers fall back to the configured default (for examplestructure.rename_fieldsforrename-all), the same as when the annotation is absent:This covers every
parse_atomuser (rename-all,rename-variant-name-fields,rename-associated-constant). An empty value (cbindgen:rename-all=) still maps toT::default()as before.Fixes the
rename-allpanic from #1184. The other reproducers in that issue (generic arity mismatches, malformed lock file, recursive#[path]module) are separate and not touched here, so the issue should stay open.How tested
parse_atom_validandparse_atom_invalidinsrc/bindgen/ir/annotation.rs.parse_atom_invalidpanicked before the fix and passes after.cbindgen --lang c: it panicked before, and now prints the warning and generates the header.CBINDGEN_TEST_VERIFY=1 cargo +nightly test: unit tests,profileand all 161tests/rustcases pass with no expectation changes. I don't have Cython or CMake locally, so I ran the expectation suite withCBINDGEN_TEST_NO_COMPILE=1, and thedepfiletests (which need CMake) could not run.cargo +stable fmt --checkandcargo +stable clippy --workspace -- -D warnings: clean.AI disclosure
This change was drafted with Claude Code, then reviewed and tested by me.
🤖 Generated with Claude Code