Skip to content

Commit b4bc4f0

Browse files
authored
Work around upstream compiler bug (#956)
Closes #716. Ref georust/geo#1255 (comment). Note that this is the same underlying implementation as upstream geo in <georust/geo#1255>. However, the trait-based implementation hits this compiler regression <rust-lang/rust#128887>, <rust-lang/rust#131960>, which prevents from compiling in release mode on a stable Rust version. For some reason, the **function-based implementation** does not hit this regression, and thus allows building geoarrow without using latest nightly and a custom `RUSTFLAGS`. Note that it's only `GeometryTrait` and `GeometryCollectionTrait` that hit this compiler bug. Other traits can use the upstream impls.
1 parent d61e866 commit b4bc4f0

File tree

12 files changed

+79
-59
lines changed

12 files changed

+79
-59
lines changed

.github/workflows/ci.yml

+3-5
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,7 @@ jobs:
103103
# - uses: actions/checkout@v4
104104
# with:
105105
# submodules: "recursive"
106-
# # We use nightly for now so that we can pass RUSTFLAGS below to work around
107-
# # https://github.com/geoarrow/geoarrow-rs/issues/716
108-
# - uses: dtolnay/rust-toolchain@nightly
106+
# - uses: dtolnay/rust-toolchain@stable
109107
# - uses: Swatinem/rust-cache@v2
110108
# - uses: prefix-dev/setup-pixi@v0.8.1
111109
# with:
@@ -118,6 +116,6 @@ jobs:
118116
# echo "PKG_CONFIG_PATH=$(pwd)/build/.pixi/envs/default/lib/pkgconfig" >> "$GITHUB_ENV"
119117
# echo "LD_LIBRARY_PATH=$(pwd)/build/.pixi/envs/default/lib" >> "$GITHUB_ENV"
120118
# - name: Build benchmarks with no features
121-
# run: RUSTFLAGS="-Zinline-mir=no" cargo bench --no-run
119+
# run: cargo bench --no-run
122120
# - name: Build benchmarks with all features
123-
# run: RUSTFLAGS="-Zinline-mir=no" cargo bench --no-run --all-features
121+
# run: cargo bench --no-run --all-features

.github/workflows/python-core-wheels.yml

-13
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ jobs:
4545
python-version: 3.x
4646
- name: Build wheels
4747
uses: PyO3/maturin-action@v1
48-
env:
49-
RUSTFLAGS: "-Zinline-mir=no"
5048
with:
51-
rust-toolchain: nightly
5249
target: ${{ matrix.platform.target }}
5350
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 --manifest-path python/${{ matrix.module }}/Cargo.toml
5451
sccache: "true"
@@ -83,8 +80,6 @@ jobs:
8380
# python-version: 3.x
8481
# - name: Build wheels
8582
# uses: PyO3/maturin-action@v1
86-
# env:
87-
# RUSTFLAGS: "-Zinline-mir=no"
8883
# with:
8984
# target: ${{ matrix.platform.target }}
9085
# args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 --manifest-path python/${{ matrix.module }}/Cargo.toml
@@ -116,10 +111,7 @@ jobs:
116111
architecture: ${{ matrix.platform.target }}
117112
- name: Build wheels
118113
uses: PyO3/maturin-action@v1
119-
env:
120-
RUSTFLAGS: "-Zinline-mir=no"
121114
with:
122-
rust-toolchain: nightly
123115
target: ${{ matrix.platform.target }}
124116
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 --manifest-path python/${{ matrix.module }}/Cargo.toml
125117
sccache: "true"
@@ -149,10 +141,7 @@ jobs:
149141
python-version: 3.x
150142
- name: Build wheels
151143
uses: PyO3/maturin-action@v1
152-
env:
153-
RUSTFLAGS: "-Zinline-mir=no"
154144
with:
155-
rust-toolchain: nightly
156145
target: ${{ matrix.platform.target }}
157146
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 --manifest-path python/${{ matrix.module }}/Cargo.toml
158147
sccache: "true"
@@ -191,8 +180,6 @@ jobs:
191180
- run: pip install pyodide-build
192181
- name: Build wheels
193182
uses: PyO3/maturin-action@v1
194-
env:
195-
RUSTFLAGS: "-Zinline-mir=no"
196183
with:
197184
rust-toolchain: nightly
198185
target: ${{ matrix.platform.target }}

.github/workflows/python-io-wheels.yml

-13
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,7 @@ jobs:
5050

5151
- name: Build wheels
5252
uses: PyO3/maturin-action@v1
53-
env:
54-
RUSTFLAGS: "-Zinline-mir=no"
5553
with:
56-
rust-toolchain: nightly
5754
target: ${{ matrix.platform.target }}
5855
# As of Nov 2024, it was necessary to manually specify -i 3.13 to get
5956
# maturin to find the executable. --find-interpreter did not find it.
@@ -88,10 +85,7 @@ jobs:
8885

8986
- name: Build wheels - ${{ matrix.platform.target }}
9087
uses: PyO3/maturin-action@v1
91-
env:
92-
RUSTFLAGS: "-Zinline-mir=no"
9388
with:
94-
rust-toolchain: nightly
9589
target: ${{ matrix.platform.target }}
9690
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 -m python/geoarrow-io/Cargo.toml
9791
sccache: "true"
@@ -124,10 +118,7 @@ jobs:
124118

125119
- name: Build wheels
126120
uses: PyO3/maturin-action@v1
127-
env:
128-
RUSTFLAGS: "-Zinline-mir=no"
129121
with:
130-
rust-toolchain: nightly
131122
target: ${{ matrix.target }}
132123
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -m python/geoarrow-io/Cargo.toml
133124

@@ -162,10 +153,8 @@ jobs:
162153
# - name: Build wheels
163154
# uses: PyO3/maturin-action@v1
164155
# with:
165-
# rust-toolchain: nightly
166156
# target: ${{ matrix.target }}
167157
# manylinux: musllinux_1_2
168-
# TODO: update rustflags env
169158
# args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 -m python/geoarrow-io/Cargo.toml
170159

171160
# - name: Install built wheel
@@ -206,10 +195,8 @@ jobs:
206195
# - name: Build wheels
207196
# uses: PyO3/maturin-action@v1
208197
# with:
209-
# rust-toolchain: nightly
210198
# target: ${{ matrix.platform.target }}
211199
# manylinux: musllinux_1_2
212-
# TODO: update rustflags env
213200
# args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 -i 3.13 -m python/geoarrow-io/Cargo.toml
214201

215202
# - uses: uraimo/run-on-arch-action@v2.5.1

python/DEVELOP.md

+2-6
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,17 @@ PYODIDE_EMSCRIPTEN_VERSION=$(pyodide config get emscripten_version)
8383
source ~/github/emscripten-core/emsdk/emsdk_env.sh
8484
```
8585

86-
Note that the addition of `RUSTFLAGS="-Zinline-mir=no"` is temporary due to https://github.com/rust-lang/rust/issues/128887.
87-
8886
Build `geoarrow-rust-core` and `geoarrow-rust-io`:
8987

9088
```bash
91-
RUSTFLAGS="-Zinline-mir=no" RUSTUP_TOOLCHAIN=nightly \
92-
maturin build \
89+
maturin build \
9390
--release \
9491
--no-default-features \
9592
-o dist \
9693
-m geoarrow-core/Cargo.toml \
9794
--target wasm32-unknown-emscripten \
9895
-i python3.11
99-
RUSTFLAGS="-Zinline-mir=no" RUSTUP_TOOLCHAIN=nightly \
100-
maturin build \
96+
maturin build \
10197
--release \
10298
--no-default-features \
10399
-o dist \

python/geoarrow-core/DEVELOP.md

+1-5
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,10 @@ source emsdk_env.sh
2727
cd ..
2828
```
2929

30-
- The `RUSTFLAGS` is temporary to get around this compiler bug.
31-
- You must use rust nightly
3230
- You must use `--no-default-features` to remove any async support. `tokio` does not compile for emscripten.
3331

3432
```bash
35-
RUSTFLAGS='-Zinline-mir=no' /
36-
RUSTUP_TOOLCHAIN=nightly /
37-
maturin build /
33+
maturin build /
3834
--no-default-features /
3935
--release /
4036
-o dist /

rust/geoarrow/src/algorithm/geo/contains.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use crate::algorithm::native::{Binary, Unary};
22
use crate::array::*;
33
use crate::datatypes::NativeType;
44
use crate::error::GeoArrowError;
5+
use crate::io::geo::geometry_to_geo;
56
use crate::trait_::NativeScalar;
67
use crate::NativeArray;
78
use arrow_array::BooleanArray;
89
use geo::Contains as _Contains;
9-
use geo_traits::to_geo::*;
1010
use geo_traits::GeometryTrait;
1111

1212
/// Checks if `rhs` is completely contained within `self`.
@@ -135,7 +135,7 @@ pub trait ContainsGeometry<Rhs> {
135135

136136
impl<G: GeometryTrait<T = f64>> ContainsGeometry<G> for PointArray {
137137
fn contains(&self, rhs: &G) -> BooleanArray {
138-
let rhs = rhs.to_geometry();
138+
let rhs = geometry_to_geo(rhs);
139139
self.try_unary_boolean::<_, GeoArrowError>(|geom| Ok(geom.to_geo().contains(&rhs)))
140140
.unwrap()
141141
}
@@ -145,7 +145,7 @@ macro_rules! impl_contains_point {
145145
($array:ty) => {
146146
impl<G: GeometryTrait<T = f64>> ContainsGeometry<G> for $array {
147147
fn contains(&self, rhs: &G) -> BooleanArray {
148-
let rhs = rhs.to_geometry();
148+
let rhs = geometry_to_geo(rhs);
149149
self.try_unary_boolean::<_, GeoArrowError>(|geom| {
150150
Ok(geom.to_geo_geometry().contains(&rhs))
151151
})

rust/geoarrow/src/algorithm/geo/intersects.rs

+9-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::chunked_array::ChunkedArray;
22
use crate::indexed::array::*;
33
use crate::indexed::chunked::*;
4+
use crate::io::geo::{geometry_collection_to_geo, geometry_to_geo};
45
use crate::trait_::NativeScalar;
56
use arrow_array::BooleanArray;
67
use geo::{BoundingRect, Intersects as _Intersects};
@@ -592,7 +593,7 @@ impl<G: GeometryTrait<T = f64>> IntersectsGeometry<G> for IndexedPointArray {
592593
type Output = BooleanArray;
593594

594595
fn intersects(&self, rhs: &G) -> Self::Output {
595-
let rhs = rhs.to_geometry();
596+
let rhs = geometry_to_geo(rhs);
596597
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
597598
geom.to_geo().intersects(&rhs)
598599
})
@@ -605,7 +606,7 @@ macro_rules! impl_intersects {
605606
type Output = BooleanArray;
606607

607608
fn intersects(&self, rhs: &G) -> Self::Output {
608-
let rhs = rhs.to_geometry();
609+
let rhs = geometry_to_geo(rhs);
609610
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
610611
geom.to_geo().intersects(&rhs)
611612
})
@@ -626,7 +627,7 @@ impl<G: GeometryTrait<T = f64>> IntersectsGeometry<G> for IndexedChunkedPointArr
626627
type Output = ChunkedArray<BooleanArray>;
627628

628629
fn intersects(&self, rhs: &G) -> Self::Output {
629-
let rhs = rhs.to_geometry();
630+
let rhs = geometry_to_geo(rhs);
630631
self.map(|chunk| IntersectsGeometry::intersects(chunk, &rhs))
631632
.try_into()
632633
.unwrap()
@@ -639,7 +640,7 @@ macro_rules! impl_intersects {
639640
type Output = ChunkedArray<BooleanArray>;
640641

641642
fn intersects(&self, rhs: &G) -> Self::Output {
642-
let rhs = rhs.to_geometry();
643+
let rhs = geometry_to_geo(rhs);
643644
self.map(|chunk| IntersectsGeometry::intersects(chunk, &rhs))
644645
.try_into()
645646
.unwrap()
@@ -666,7 +667,7 @@ impl<G: GeometryCollectionTrait<T = f64>> IntersectsGeometryCollection<G> for In
666667
type Output = BooleanArray;
667668

668669
fn intersects(&self, rhs: &G) -> Self::Output {
669-
let rhs = rhs.to_geometry_collection();
670+
let rhs = geometry_collection_to_geo(rhs);
670671
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
671672
geom.to_geo().intersects(&rhs)
672673
})
@@ -679,7 +680,7 @@ macro_rules! impl_intersects {
679680
type Output = BooleanArray;
680681

681682
fn intersects(&self, rhs: &G) -> Self::Output {
682-
let rhs = rhs.to_geometry_collection();
683+
let rhs = geometry_collection_to_geo(rhs);
683684
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
684685
geom.to_geo().intersects(&rhs)
685686
})
@@ -702,7 +703,7 @@ impl<G: GeometryCollectionTrait<T = f64>> IntersectsGeometryCollection<G>
702703
type Output = ChunkedArray<BooleanArray>;
703704

704705
fn intersects(&self, rhs: &G) -> Self::Output {
705-
let rhs = rhs.to_geometry_collection();
706+
let rhs = geometry_collection_to_geo(rhs);
706707
self.map(|chunk| IntersectsGeometryCollection::intersects(chunk, &rhs))
707708
.try_into()
708709
.unwrap()
@@ -715,7 +716,7 @@ macro_rules! impl_intersects {
715716
type Output = ChunkedArray<BooleanArray>;
716717

717718
fn intersects(&self, rhs: &G) -> Self::Output {
718-
let rhs = rhs.to_geometry_collection();
719+
let rhs = geometry_collection_to_geo(rhs);
719720
self.map(|chunk| IntersectsGeometryCollection::intersects(chunk, &rhs))
720721
.try_into()
721722
.unwrap()

rust/geoarrow/src/io/geo.rs

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
//! Convert structs that implement geo-traits to [geo-types] objects.
2+
//!
3+
//! Note that this is the same underlying implementation as upstream [geo] in
4+
//! <https://github.com/georust/geo/pull/1255>. However, the trait-based implementation hits this
5+
//! compiler regression <https://github.com/rust-lang/rust/issues/128887>,
6+
//! <https://github.com/rust-lang/rust/issues/131960>, which prevents from compiling in release
7+
//! mode on a stable Rust version. For some reason, the **function-based implementation** does not
8+
//! hit this regression, and thus allows building geoarrow without using latest nightly and a
9+
//! custom `RUSTFLAGS`.
10+
//!
11+
//! Note that it's only `GeometryTrait` and `GeometryCollectionTrait` that hit this compiler bug.
12+
//! Other traits can use the upstream impls.
13+
14+
use geo::{CoordNum, Geometry, GeometryCollection};
15+
16+
use geo_traits::to_geo::{
17+
ToGeoLine, ToGeoLineString, ToGeoMultiLineString, ToGeoMultiPoint, ToGeoMultiPolygon,
18+
ToGeoPoint, ToGeoPolygon, ToGeoRect, ToGeoTriangle,
19+
};
20+
use geo_traits::{GeometryCollectionTrait, GeometryTrait, GeometryType};
21+
22+
/// Convert any Geometry to a [`Geometry`].
23+
///
24+
/// Only the first two dimensions will be kept.
25+
pub fn geometry_to_geo<T: CoordNum>(geometry: &impl GeometryTrait<T = T>) -> Geometry<T> {
26+
use GeometryType::*;
27+
28+
match geometry.as_type() {
29+
Point(geom) => Geometry::Point(geom.to_point()),
30+
LineString(geom) => Geometry::LineString(geom.to_line_string()),
31+
Polygon(geom) => Geometry::Polygon(geom.to_polygon()),
32+
MultiPoint(geom) => Geometry::MultiPoint(geom.to_multi_point()),
33+
MultiLineString(geom) => Geometry::MultiLineString(geom.to_multi_line_string()),
34+
MultiPolygon(geom) => Geometry::MultiPolygon(geom.to_multi_polygon()),
35+
GeometryCollection(geom) => Geometry::GeometryCollection(geometry_collection_to_geo(geom)),
36+
Rect(geom) => Geometry::Rect(geom.to_rect()),
37+
Line(geom) => Geometry::Line(geom.to_line()),
38+
Triangle(geom) => Geometry::Triangle(geom.to_triangle()),
39+
}
40+
}
41+
42+
/// Convert any GeometryCollection to a [`GeometryCollection`].
43+
///
44+
/// Only the first two dimensions will be kept.
45+
pub fn geometry_collection_to_geo<T: CoordNum>(
46+
geometry_collection: &impl GeometryCollectionTrait<T = T>,
47+
) -> GeometryCollection<T> {
48+
GeometryCollection::new_from(
49+
geometry_collection
50+
.geometries()
51+
.map(|geometry| geometry_to_geo(&geometry))
52+
.collect(),
53+
)
54+
}

rust/geoarrow/src/io/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub(crate) mod display;
1111
pub mod flatgeobuf;
1212
#[cfg(feature = "gdal")]
1313
pub mod gdal;
14+
pub(crate) mod geo;
1415
pub mod geojson;
1516
pub mod geojson_lines;
1617
#[cfg(feature = "geos")]

rust/geoarrow/src/scalar/binary/scalar.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::error::Result;
2+
use crate::io::geo::geometry_to_geo;
23
use crate::trait_::NativeScalar;
34
use arrow_array::{GenericBinaryArray, OffsetSizeTrait};
45
use geo::BoundingRect;
5-
use geo_traits::to_geo::ToGeoGeometry;
66
use geo_traits::GeometryTrait;
77
use rstar::{RTreeObject, AABB};
88

@@ -77,7 +77,7 @@ impl<O: OffsetSizeTrait> AsRef<[u8]> for WKB<'_, O> {
7777

7878
impl<O: OffsetSizeTrait> From<&WKB<'_, O>> for geo::Geometry {
7979
fn from(value: &WKB<'_, O>) -> Self {
80-
value.parse().unwrap().to_geometry()
80+
geometry_to_geo(&value.parse().unwrap())
8181
}
8282
}
8383

rust/geoarrow/src/scalar/geometry/scalar.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::algorithm::native::eq::geometry_eq;
2+
use crate::io::geo::geometry_to_geo;
23
use crate::scalar::*;
34
use crate::trait_::NativeScalar;
4-
use geo_traits::to_geo::ToGeoGeometry;
55
use geo_traits::{
66
GeometryCollectionTrait, GeometryTrait, GeometryType, LineStringTrait, MultiLineStringTrait,
77
MultiPointTrait, MultiPolygonTrait, PointTrait, PolygonTrait, RectTrait, UnimplementedLine,
@@ -241,7 +241,7 @@ impl From<Geometry<'_>> for geo::Geometry {
241241

242242
impl From<&Geometry<'_>> for geo::Geometry {
243243
fn from(value: &Geometry<'_>) -> Self {
244-
ToGeoGeometry::to_geometry(value)
244+
geometry_to_geo(value)
245245
}
246246
}
247247

rust/geoarrow/src/scalar/geometrycollection/scalar.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ use crate::algorithm::native::eq::geometry_collection_eq;
22
use crate::array::util::OffsetBufferUtils;
33
use crate::array::MixedGeometryArray;
44
use crate::datatypes::Dimension;
5+
use crate::io::geo::geometry_collection_to_geo;
56
use crate::scalar::Geometry;
67
use crate::trait_::ArrayAccessor;
78
use crate::trait_::NativeScalar;
89
use crate::NativeArray;
910
use arrow_buffer::OffsetBuffer;
10-
use geo_traits::to_geo::ToGeoGeometryCollection;
1111
use geo_traits::GeometryCollectionTrait;
1212
use rstar::{RTreeObject, AABB};
1313

@@ -111,7 +111,7 @@ impl<'a> GeometryCollectionTrait for &'a GeometryCollection<'a> {
111111

112112
impl From<&GeometryCollection<'_>> for geo::GeometryCollection {
113113
fn from(value: &GeometryCollection<'_>) -> Self {
114-
value.to_geometry_collection()
114+
geometry_collection_to_geo(value)
115115
}
116116
}
117117

0 commit comments

Comments
 (0)