Canonicalize template specialization types to prevent duplicate definitions. - #1805
Open
copybara-service[bot] wants to merge 1 commit into
Open
Canonicalize template specialization types to prevent duplicate definitions.#1805copybara-service[bot] wants to merge 1 commit into
copybara-service[bot] wants to merge 1 commit into
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. |
copybara-service
Bot
force-pushed
the
test_966609744
branch
5 times, most recently
from
August 20, 2026 19:46
d3d8f22 to
195ab3a
Compare
copybara-service
Bot
force-pushed
the
test_966609744
branch
from
August 20, 2026 20:49
195ab3a to
5f1c7f4
Compare
…itions. Crubit wraps template specializations (such as `Option<T>`, `Result<T, E>`, `Vec<T>`, and tuples) in `#ifndef` preprocessor guards to prevent duplicate definition errors in Clang when multiple generated headers are included in the same translation unit. We ran into a chain of two issues with how these guards and types are generated: 1. include guards based on C++ type spellings - Problem: Previously, `#ifndef` guard names were derived from the formatted C++ type string. This was vulnerable to duplicate definition errors because two spellings of a C++ type could actually refer to the same underlying type but generate different guard macro names. When multiple headers were included together, the guards wouldn't interfere, Clang would desugar the aliases and canonicalize them to the same underlying C++ type, and then fail with redefinition errors. - Fix: Derive the `#ifndef` guard directly from the fully canonicalized Rust type (`specialization_guard_name`) using rustc's `with_no_visible_paths`, `with_no_trimmed_paths`, and `with_resolve_crate_name` flags to see through all `pub use` aliases and re-exports. 2. `usize` vs `u64` specialization clashes - Problem: Basing the `#ifndef` guard on the Rust type meant `Result<usize, ...>` and `Result<u64, ...>` generated different guard names (`..._usize_...` vs `..._u64_...`), since in Rust they are distinct types. However, commit 7ba4d1d previously changed C++ formatting (`format_ty_for_cc`) to emit `uint64_t` for `usize` in template arguments. This caused Clang to receive two identical C++ specialization definitions for `Result<uint64_t, ...>` guarded by different macro names, resulting in redefinition errors. - Fix: Canonicalize the Rust `Ty` upfront using a `TypeFolder` (`canonicalize_specialization_ty`) that rewrites `usize`/`isize` into platform fixed-width integers (`u64`/`i64`) and lifetimes to `'static`. This provides a single source of truth for in-memory deduplication, include guard generation, and C++ code emission, allowing us to clean up the ad-hoc `TypeLocation::TemplateArg` formatting checks in `format_type.rs`. Note that some of the diffs look completely wrong, but aren't. For example, result_cc_api.h has some diffs that appear to swap the Ok and Err types of Result, e.g. `Result<isize, i8>` -> `Result<i8, isize>`. But this is actually because the original Rust file used both, and the tool decided to swap the order they're emitted in and the diff tool doesn't realize the full block was swapped because they're so similar. PiperOrigin-RevId: 966609744
copybara-service
Bot
force-pushed
the
test_966609744
branch
from
August 20, 2026 21:12
5f1c7f4 to
5c2e8e1
Compare
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.
Canonicalize template specialization types to prevent duplicate definitions.
Crubit wraps template specializations (such as
Option<T>,Result<T, E>,Vec<T>, and tuples) in#ifndefpreprocessor guards to prevent duplicate definition errors in Clang when multiple generated headers are included in the same translation unit. We ran into a chain of two issues with how these guards and types are generated:#ifndefguard names were derived from the formatted C++ type string. This was vulnerable to duplicate definition errors because two spellings of a C++ type could actually refer to the same underlying type but generate different guard macro names. When multiple headers were included together, the guards wouldn't interfere, Clang would desugar the aliases and canonicalize them to the same underlying C++ type, and then fail with redefinition errors.#ifndefguard directly from the fully canonicalized Rust type (specialization_guard_name) using rustc'swith_no_visible_paths,with_no_trimmed_paths, andwith_resolve_crate_nameflags to see through allpub usealiases and re-exports.usizevsu64specialization clashes#ifndefguard on the Rust type meantResult<usize, ...>andResult<u64, ...>generated different guard names (..._usize_...vs..._u64_...), since in Rust they are distinct types. However, commit 7ba4d1d previously changed C++ formatting (format_ty_for_cc) to emituint64_tforusizein template arguments. This caused Clang to receive two identical C++ specialization definitions forResult<uint64_t, ...>guarded by different macro names, resulting in redefinition errors.Tyupfront using aTypeFolder(canonicalize_specialization_ty) that rewritesusize/isizeinto platform fixed-width integers (u64/i64) and lifetimes to'static. This provides a single source of truth for in-memory deduplication, include guard generation, and C++ code emission, allowing us to clean up the ad-hocTypeLocation::TemplateArgformatting checks informat_type.rs.Note that some of the diffs look completely wrong, but aren't. For example, result_cc_api.h has some diffs that appear to swap the Ok and Err types of Result, e.g.
Result<isize, i8>->Result<i8, isize>. But this is actually because the original Rust file used both, and the tool decided to swap the order they're emitted in and the diff tool doesn't realize the full block was swapped because they're so similar.