Skip to content

fix: Remove trait wrappers to work around Rust 1.90 compiler regression#77

Merged
kylebarron merged 8 commits intomainfrom
kyle/remove-trait-wrappers
Sep 25, 2025
Merged

fix: Remove trait wrappers to work around Rust 1.90 compiler regression#77
kylebarron merged 8 commits intomainfrom
kyle/remove-trait-wrappers

Conversation

@kylebarron
Copy link
Copy Markdown
Member

@kylebarron kylebarron commented Sep 23, 2025

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.
  • I ran cargo fmt

Rust 1.90 caused a regression in release mode for consumers of the geoarrow-array crate: geoarrow/geoarrow-rs#1339, rust-lang/rust#131960. This code compiled fine in 1.89 but no longer compiles in 1.90 in release mode. (It compiles in debug mode)

I tracked this down to the trait wrappers implemented here. If you pull main of geoarrow-rs (geoarrow/geoarrow-rs@338a08c) and run cargo +1.90 build --release -p geoarrow-array you get a compiler error. If you check out the branch in geoarrow/geoarrow-rs#1341 you no longer get a compiler error.

Perhaps we should add some docs to the code to explain why we're not using trait wrappers? Then again, this implementation is so simple that I almost think I should've used this from the start.

Copy link
Copy Markdown
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

Sad but necessary

@kylebarron
Copy link
Copy Markdown
Member Author

I liked the "elegance" of the LineWrapper/TriangleWrapper/RectWrapper in not making developers think extra about those cases while still fleshing out all the geometry types defined by geo, but alas 😕

Kontinuation added a commit to wherobots/geo that referenced this pull request Sep 25, 2025
… Rust compiler bug in 1.90.0 (#11)

This patch removes the GAT `GeometryCollectionTypeExt` from `GeometryTraitExt` because it would introduce recursive GATs such as `G::GeometryCollectionTypeExt::GeometryTypeExt::GeometryCollectionTypeExt::...` and easily
trigger a Rust compiler bug: rust-lang/rust#128887 and rust-lang/rust#131960. See also geoarrow/geoarrow-rs#1339.

Although this could be worked around by not implementing generic functions using trait-based approach and use
function-based approach instead, see geoarrow/geoarrow-rs#956 and georust/wkb#77, we are not certain if there will be other issues caused by recursive GATs in the future. So we decided to completely get rid of recursive GATs.
@kylebarron kylebarron merged commit 6fd747d into main Sep 25, 2025
1 check passed
@kylebarron kylebarron deleted the kyle/remove-trait-wrappers branch September 25, 2025 04:22
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.

2 participants