This repository was archived by the owner on Sep 21, 2026. It is now read-only.
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Choochmeque
added a commit
to Choochmeque/autocxx
that referenced
this pull request
Sep 5, 2026
* Bump tracing-subscriber to 0.3.23 tracing-subscriber before 0.3.20 passes log data straight through to the terminal, so a record containing ANSI escape sequences can move the cursor, recolour, or overwrite surrounding output (RUSTSEC-2025-0055). It reaches us as a dev-dependency of test-log, which every crate in the workspace uses, so the lockfile pinned the vulnerable version for anyone running our tests. Take the current 0.3.23 rather than the 0.3.20 of the original PR; the resolver pulls matchers, nu-ansi-term, tracing and tracing-core along with it and drops overload and the old regex-automata. Ported from google/autocxx#1495 by dependabot[bot]. * Include <cstdint> in the tests that use uint32_t Four tests write uint32_t in their header but include only <string> and <memory>. Those headers happen to drag in <cstdint> on the standard library implementations we test against today, which is not something either one promises; on a libstdc++ that stops doing so the four headers fail to compile and the tests fail for a reason that has nothing to do with what they are testing. Include <cstdint> directly, in the position the neighbouring tests use. Ported from google/autocxx#1487 by tchebb. * Fix documentation and spelling nits Four unrelated small things: - is_known_subtitute_type was missing an 's'; renamed, along with its one call site. - The deprecated is_subclass re-export sat above the doc comment written for subclass, so rustdoc attached that whole comment to the deprecated alias and left subclass undocumented. Move the alias below. - Doc comments wrote the attribute macro as #[`is_subclass`], which rustdoc renders as a literal followed by a stray '#' and links nowhere - and names the deprecated alias besides. Write it as [`#[subclass]`][subclass]. - contributing.md offered a cargo invocation naming a test (test_cycle_string_full_pipeline) that no longer exists, under the old --all spelling. Point it at integration_test::test_cycle_string with --exact, which is what someone following the instructions needs today. The original PR also fixed a 'reduncancy' typo in parse_bindgen.rs; that comment is already gone from this fork. Ported from google/autocxx#1477 by tchebb. * Allow pointer-to-pointer fields in POD structs ensure_pointee_is_valid refused a pointer whose pointee is another pointer wherever it appeared, so a struct with a `float** data` member could not be generated at all: the whole struct was discarded with InvalidPointerPointee, taking every function that mentions it with it. The refusal is right for a function parameter or return, where autocxx would have to decide what the outer pointer means in Rust. A struct field needs no such decision - it is data whose layout we copy, and Rust sees the field as a raw pointer either way - so make the check take the conversion context and accept a pointer pointee only inside a struct field. The pointee itself is still converted as WithinReference, so `float***` remains refused, and pointer-to-pointer parameters are unaffected (test_pointer_to_pointer still passes). The fork carries a third convert_ptr call site the original PR did not, the typedef-resolves-to-pointer branch added for google/autocxx#1368; it threads the caller's context through like the others. Fixes google/autocxx#1278. Ported from google/autocxx#1511 by lukiod. * Stop generated cxx re-exports warning as unused imports Every item autocxx puts in the output mod arrives there as a `pub use` out of the private cxxbridge mod, and that mod is normally private to the caller's crate, so a `pub use` nobody happens to name is not reachable and rustc reports it as an unused import. The caller cannot do anything about it: the re-export is the only name they have for the item, and using part of a generated API but not all of it is not a mistake. Add #[allow(unused_imports)], as the bindgen-side re-exports in generate_bindgen_use_stmt already do. The original PR was after one case of this - an abstract type appearing only in a function signature, whose only real use is the one cxx emits against the cxxbridge binding. That exact case no longer reproduces here: this fork emits a Drop or MakeCppStorage impl naming `output::<type>` for such types, which keeps the type's re-export used. The same helper's re-exports for functions do still warn, which is what the new test pins by building the generated crate under deny(unused_imports). Ported from google/autocxx#1486 by tchebb. * Give generated unsafe operations their own unsafe blocks RFC 2585 says an unsafe operation inside an `unsafe fn` needs its own `unsafe` block; the function's signature is not blanket permission, and the `unsafe_op_in_unsafe_fn` lint is on its way to becoming an error. Generated code leaned on the signature and carried a blanket `#[allow(unsafe_op_in_unsafe_fn)]` over the whole mod so it would compile - which also silenced the lint for anyone denying it in their own crate. Treat the surrounding context as safe when emitting a call body, so every unsafe operation gets its own block, and drop the blanket allow. `trait_call_is_unsafe` existed only to widen that same context and was `false` at all four of its construction sites, so it goes too. Deviation from the original PR: it also set bindgen's `wrap_unsafe_ops(true)`, which cannot be used here. That option makes bindgen emit a wrapper around each extern fn, derived from a name our `item_name` parse callback has already suffixed with `_bindgen_original`, so the wrapper arrives as `create_bindgen_original_bindgen_original`; `strip_bindgen_original_suffix` takes one suffix off, the name no longer matches the method it belongs to, and static methods quietly become free functions (`test_conflicting_static_functions` fails). Instead the allow moves onto bindgen's own mod, where the comment records why, leaving the code autocxx writes itself under the lint. The new test builds a generated crate under deny(unsafe_op_in_unsafe_fn) and deny(unused_unsafe), which catches both a missing block and a redundant one. Ported from google/autocxx#1476 by xinchengxx. * Add pretty!() to lay the generated Rust out readably The .rs file autocxx writes is a single line of tokens, which is fine for the compiler and useless to anyone opening it to see what was generated - the usual reason to set AUTOCXX_RS_FILE at all. Add a pretty!() directive which runs the mod through prettyplease, already a dependency for the same purpose in the engine's own logging. It stays off by default: the pass costs build time and buys nothing for a file only rustc will read. The @generated marker is still prepended afterwards, so it keeps the first line either way, and prettyplease is deterministic for a given resolved version, so the reproducibility the marker's test pins is unaffected. (A prettyplease upgrade may legitimately move whitespace; that is a formatter change, not nondeterminism.) RsOutput now carries the mod itself rather than a rendered token stream, so it can be rendered either way at the end. Ported from google/autocxx#1497 by qthree. * Qualify the type when calling a nested struct's static method Calling a static method on a struct nested inside another struct emitted C++ naming the bindgen type rather than the C++ one: for `struct A { struct B { static void f(); }; };` the wrapper read `A_B::f()`, and `outer::A_B::f()` when a namespace was involved. Neither names anything, so the generated C++ failed to compile. Ask the name map for the type's C++ spelling, as the destructor arm just above already does. It restores the nesting and carries the namespace itself, so the namespace is no longer joined on separately. Ported from google/autocxx#1496 by qthree, which rebased the abandoned google/autocxx#1380. Its fix reached for the original-name map directly; this uses the existing helper for the same lookup.
This branch has not been deployed
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Based on #1380
Original PR generated invalid code, I've made corrections, now integration_tests are passing locally, except for:
but they don't pass on main branch either.