Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around upstream compiler bug #956

Merged
merged 4 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,7 @@ jobs:
# - uses: actions/checkout@v4
# with:
# submodules: "recursive"
# # We use nightly for now so that we can pass RUSTFLAGS below to work around
# # https://github.com/geoarrow/geoarrow-rs/issues/716
# - uses: dtolnay/rust-toolchain@nightly
# - uses: dtolnay/rust-toolchain@stable
# - uses: Swatinem/rust-cache@v2
# - uses: prefix-dev/[email protected]
# with:
Expand All @@ -118,6 +116,6 @@ jobs:
# echo "PKG_CONFIG_PATH=$(pwd)/build/.pixi/envs/default/lib/pkgconfig" >> "$GITHUB_ENV"
# echo "LD_LIBRARY_PATH=$(pwd)/build/.pixi/envs/default/lib" >> "$GITHUB_ENV"
# - name: Build benchmarks with no features
# run: RUSTFLAGS="-Zinline-mir=no" cargo bench --no-run
# run: cargo bench --no-run
# - name: Build benchmarks with all features
# run: RUSTFLAGS="-Zinline-mir=no" cargo bench --no-run --all-features
# run: cargo bench --no-run --all-features
13 changes: 0 additions & 13 deletions .github/workflows/python-core-wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,7 @@ jobs:
python-version: 3.x
- name: Build wheels
uses: PyO3/maturin-action@v1
env:
RUSTFLAGS: "-Zinline-mir=no"
with:
rust-toolchain: nightly
target: ${{ matrix.platform.target }}
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
sccache: "true"
Expand Down Expand Up @@ -83,8 +80,6 @@ jobs:
# python-version: 3.x
# - name: Build wheels
# uses: PyO3/maturin-action@v1
# env:
# RUSTFLAGS: "-Zinline-mir=no"
# with:
# target: ${{ matrix.platform.target }}
# 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
Expand Down Expand Up @@ -116,10 +111,7 @@ jobs:
architecture: ${{ matrix.platform.target }}
- name: Build wheels
uses: PyO3/maturin-action@v1
env:
RUSTFLAGS: "-Zinline-mir=no"
with:
rust-toolchain: nightly
target: ${{ matrix.platform.target }}
args: --release --out dist -i 3.9 -i 3.10 -i 3.11 -i 3.12 --manifest-path python/${{ matrix.module }}/Cargo.toml
sccache: "true"
Expand Down Expand Up @@ -149,10 +141,7 @@ jobs:
python-version: 3.x
- name: Build wheels
uses: PyO3/maturin-action@v1
env:
RUSTFLAGS: "-Zinline-mir=no"
with:
rust-toolchain: nightly
target: ${{ matrix.platform.target }}
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
sccache: "true"
Expand Down Expand Up @@ -191,8 +180,6 @@ jobs:
- run: pip install pyodide-build
- name: Build wheels
uses: PyO3/maturin-action@v1
env:
RUSTFLAGS: "-Zinline-mir=no"
with:
rust-toolchain: nightly
target: ${{ matrix.platform.target }}
Expand Down
13 changes: 0 additions & 13 deletions .github/workflows/python-io-wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ jobs:

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

- name: Build wheels - ${{ matrix.platform.target }}
uses: PyO3/maturin-action@v1
env:
RUSTFLAGS: "-Zinline-mir=no"
with:
rust-toolchain: nightly
target: ${{ matrix.platform.target }}
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
sccache: "true"
Expand Down Expand Up @@ -124,10 +118,7 @@ jobs:

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

Expand Down Expand Up @@ -162,10 +153,8 @@ jobs:
# - name: Build wheels
# uses: PyO3/maturin-action@v1
# with:
# rust-toolchain: nightly
# target: ${{ matrix.target }}
# manylinux: musllinux_1_2
# TODO: update rustflags env
# 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

# - name: Install built wheel
Expand Down Expand Up @@ -206,10 +195,8 @@ jobs:
# - name: Build wheels
# uses: PyO3/maturin-action@v1
# with:
# rust-toolchain: nightly
# target: ${{ matrix.platform.target }}
# manylinux: musllinux_1_2
# TODO: update rustflags env
# 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

# - uses: uraimo/[email protected]
Expand Down
8 changes: 2 additions & 6 deletions python/DEVELOP.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,17 @@ PYODIDE_EMSCRIPTEN_VERSION=$(pyodide config get emscripten_version)
source ~/github/emscripten-core/emsdk/emsdk_env.sh
```

Note that the addition of `RUSTFLAGS="-Zinline-mir=no"` is temporary due to https://github.com/rust-lang/rust/issues/128887.

Build `geoarrow-rust-core` and `geoarrow-rust-io`:

```bash
RUSTFLAGS="-Zinline-mir=no" RUSTUP_TOOLCHAIN=nightly \
maturin build \
maturin build \
--release \
--no-default-features \
-o dist \
-m geoarrow-core/Cargo.toml \
--target wasm32-unknown-emscripten \
-i python3.11
RUSTFLAGS="-Zinline-mir=no" RUSTUP_TOOLCHAIN=nightly \
maturin build \
maturin build \
--release \
--no-default-features \
-o dist \
Expand Down
6 changes: 1 addition & 5 deletions python/geoarrow-core/DEVELOP.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,10 @@ source emsdk_env.sh
cd ..
```

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

```bash
RUSTFLAGS='-Zinline-mir=no' /
RUSTUP_TOOLCHAIN=nightly /
maturin build /
maturin build /
--no-default-features /
--release /
-o dist /
Expand Down
6 changes: 3 additions & 3 deletions rust/geoarrow/src/algorithm/geo/contains.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use crate::algorithm::native::{Binary, Unary};
use crate::array::*;
use crate::datatypes::NativeType;
use crate::error::GeoArrowError;
use crate::io::geo::geometry_to_geo;
use crate::trait_::NativeScalar;
use crate::NativeArray;
use arrow_array::BooleanArray;
use geo::Contains as _Contains;
use geo_traits::to_geo::*;
use geo_traits::GeometryTrait;

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

impl<G: GeometryTrait<T = f64>> ContainsGeometry<G> for PointArray {
fn contains(&self, rhs: &G) -> BooleanArray {
let rhs = rhs.to_geometry();
let rhs = geometry_to_geo(rhs);
self.try_unary_boolean::<_, GeoArrowError>(|geom| Ok(geom.to_geo().contains(&rhs)))
.unwrap()
}
Expand All @@ -145,7 +145,7 @@ macro_rules! impl_contains_point {
($array:ty) => {
impl<G: GeometryTrait<T = f64>> ContainsGeometry<G> for $array {
fn contains(&self, rhs: &G) -> BooleanArray {
let rhs = rhs.to_geometry();
let rhs = geometry_to_geo(rhs);
self.try_unary_boolean::<_, GeoArrowError>(|geom| {
Ok(geom.to_geo_geometry().contains(&rhs))
})
Expand Down
17 changes: 9 additions & 8 deletions rust/geoarrow/src/algorithm/geo/intersects.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::chunked_array::ChunkedArray;
use crate::indexed::array::*;
use crate::indexed::chunked::*;
use crate::io::geo::{geometry_collection_to_geo, geometry_to_geo};
use crate::trait_::NativeScalar;
use arrow_array::BooleanArray;
use geo::{BoundingRect, Intersects as _Intersects};
Expand Down Expand Up @@ -592,7 +593,7 @@ impl<G: GeometryTrait<T = f64>> IntersectsGeometry<G> for IndexedPointArray {
type Output = BooleanArray;

fn intersects(&self, rhs: &G) -> Self::Output {
let rhs = rhs.to_geometry();
let rhs = geometry_to_geo(rhs);
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
geom.to_geo().intersects(&rhs)
})
Expand All @@ -605,7 +606,7 @@ macro_rules! impl_intersects {
type Output = BooleanArray;

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

fn intersects(&self, rhs: &G) -> Self::Output {
let rhs = rhs.to_geometry();
let rhs = geometry_to_geo(rhs);
self.map(|chunk| IntersectsGeometry::intersects(chunk, &rhs))
.try_into()
.unwrap()
Expand All @@ -639,7 +640,7 @@ macro_rules! impl_intersects {
type Output = ChunkedArray<BooleanArray>;

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

fn intersects(&self, rhs: &G) -> Self::Output {
let rhs = rhs.to_geometry_collection();
let rhs = geometry_collection_to_geo(rhs);
self.unary_boolean(&rhs.bounding_rect().unwrap(), |geom| {
geom.to_geo().intersects(&rhs)
})
Expand All @@ -679,7 +680,7 @@ macro_rules! impl_intersects {
type Output = BooleanArray;

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

fn intersects(&self, rhs: &G) -> Self::Output {
let rhs = rhs.to_geometry_collection();
let rhs = geometry_collection_to_geo(rhs);
self.map(|chunk| IntersectsGeometryCollection::intersects(chunk, &rhs))
.try_into()
.unwrap()
Expand All @@ -715,7 +716,7 @@ macro_rules! impl_intersects {
type Output = ChunkedArray<BooleanArray>;

fn intersects(&self, rhs: &G) -> Self::Output {
let rhs = rhs.to_geometry_collection();
let rhs = geometry_collection_to_geo(rhs);
self.map(|chunk| IntersectsGeometryCollection::intersects(chunk, &rhs))
.try_into()
.unwrap()
Expand Down
54 changes: 54 additions & 0 deletions rust/geoarrow/src/io/geo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//! Convert structs that implement geo-traits to [geo-types] objects.
//!
//! Note that this is the same underlying implementation as upstream [geo] in
//! <https://github.com/georust/geo/pull/1255>. However, the trait-based implementation hits this
//! compiler regression <https://github.com/rust-lang/rust/issues/128887>,
//! <https://github.com/rust-lang/rust/issues/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.

use geo::{CoordNum, Geometry, GeometryCollection};

use geo_traits::to_geo::{
ToGeoLine, ToGeoLineString, ToGeoMultiLineString, ToGeoMultiPoint, ToGeoMultiPolygon,
ToGeoPoint, ToGeoPolygon, ToGeoRect, ToGeoTriangle,
};
use geo_traits::{GeometryCollectionTrait, GeometryTrait, GeometryType};

/// Convert any Geometry to a [`Geometry`].
///
/// Only the first two dimensions will be kept.
pub fn geometry_to_geo<T: CoordNum>(geometry: &impl GeometryTrait<T = T>) -> Geometry<T> {
use GeometryType::*;

match geometry.as_type() {
Point(geom) => Geometry::Point(geom.to_point()),
LineString(geom) => Geometry::LineString(geom.to_line_string()),
Polygon(geom) => Geometry::Polygon(geom.to_polygon()),
MultiPoint(geom) => Geometry::MultiPoint(geom.to_multi_point()),
MultiLineString(geom) => Geometry::MultiLineString(geom.to_multi_line_string()),
MultiPolygon(geom) => Geometry::MultiPolygon(geom.to_multi_polygon()),
GeometryCollection(geom) => Geometry::GeometryCollection(geometry_collection_to_geo(geom)),
Rect(geom) => Geometry::Rect(geom.to_rect()),
Line(geom) => Geometry::Line(geom.to_line()),
Triangle(geom) => Geometry::Triangle(geom.to_triangle()),
}
}

/// Convert any GeometryCollection to a [`GeometryCollection`].
///
/// Only the first two dimensions will be kept.
pub fn geometry_collection_to_geo<T: CoordNum>(
geometry_collection: &impl GeometryCollectionTrait<T = T>,
) -> GeometryCollection<T> {
GeometryCollection::new_from(
geometry_collection
.geometries()
.map(|geometry| geometry_to_geo(&geometry))
.collect(),
)
}
1 change: 1 addition & 0 deletions rust/geoarrow/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub(crate) mod display;
pub mod flatgeobuf;
#[cfg(feature = "gdal")]
pub mod gdal;
pub(crate) mod geo;
pub mod geojson;
pub mod geojson_lines;
#[cfg(feature = "geos")]
Expand Down
4 changes: 2 additions & 2 deletions rust/geoarrow/src/scalar/binary/scalar.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::error::Result;
use crate::io::geo::geometry_to_geo;
use crate::trait_::NativeScalar;
use arrow_array::{GenericBinaryArray, OffsetSizeTrait};
use geo::BoundingRect;
use geo_traits::to_geo::ToGeoGeometry;
use geo_traits::GeometryTrait;
use rstar::{RTreeObject, AABB};

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

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

Expand Down
4 changes: 2 additions & 2 deletions rust/geoarrow/src/scalar/geometry/scalar.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::algorithm::native::eq::geometry_eq;
use crate::io::geo::geometry_to_geo;
use crate::scalar::*;
use crate::trait_::NativeScalar;
use geo_traits::to_geo::ToGeoGeometry;
use geo_traits::{
GeometryCollectionTrait, GeometryTrait, GeometryType, LineStringTrait, MultiLineStringTrait,
MultiPointTrait, MultiPolygonTrait, PointTrait, PolygonTrait, RectTrait, UnimplementedLine,
Expand Down Expand Up @@ -241,7 +241,7 @@ impl From<Geometry<'_>> for geo::Geometry {

impl From<&Geometry<'_>> for geo::Geometry {
fn from(value: &Geometry<'_>) -> Self {
ToGeoGeometry::to_geometry(value)
geometry_to_geo(value)
}
}

Expand Down
4 changes: 2 additions & 2 deletions rust/geoarrow/src/scalar/geometrycollection/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ use crate::algorithm::native::eq::geometry_collection_eq;
use crate::array::util::OffsetBufferUtils;
use crate::array::MixedGeometryArray;
use crate::datatypes::Dimension;
use crate::io::geo::geometry_collection_to_geo;
use crate::scalar::Geometry;
use crate::trait_::ArrayAccessor;
use crate::trait_::NativeScalar;
use crate::NativeArray;
use arrow_buffer::OffsetBuffer;
use geo_traits::to_geo::ToGeoGeometryCollection;
use geo_traits::GeometryCollectionTrait;
use rstar::{RTreeObject, AABB};

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

impl From<&GeometryCollection<'_>> for geo::GeometryCollection {
fn from(value: &GeometryCollection<'_>) -> Self {
value.to_geometry_collection()
geometry_collection_to_geo(value)
}
}

Expand Down
Loading