Impl Point for [RTreeNum; N] const generic N

[dominikWin]

• I agree to follow the project's code of conduct.
• I added an entry to rstar/CHANGELOG.md if knowledge of this change could be valuable to users.

---

Point is now implemented for [RTreeNum; N] where N is a usize. Previously it was only for N values between 2 and 9. Const generics are stable in every supported version of Rust.

[michaelkirk]

I think directionally this change makes sense.

One weird edge case I was thinking about - won't this newly impl Point for [S; 1] and [S; 0]?

Admittedly it probably doesn't make sense to try to use RTree with those parameters, but it would be ideal to have the compiler enforce that. IIRC someone requesting support previously tried to build an RTree with 1 dimensional points.

If it's not possible to enforce that with the compiler, maybe a runtime panic with a clear message is preferable.

[dominikWin]

Fair point. Unfortunately this is a pain point of the current const generics spec. verify_parameters currently does an assert for this, so I added one to this PR with the same message. I think this is the best solution.

The only other way I can find is to use an associated trait:

trait AssertGt1 {
    const VALID: ();
}

impl<S, const N: usize> AssertGt1 for [S; N] where S: RTreeNum {
    const VALID: () = assert!(N >= 2);
}

Then one of the functions in impl<S, const N: usize> Point for [S; N] can start with _ = <Self as AssertGt1>::VALID;. This fails with this error:

error[E0080]: evaluation of `<[f64; 1] as rstar::point::AssertGt1>::VALID` failed
   --> <path>/rstar/src/point.rs:313:23
    |
313 |     const VALID: () = assert!(N >= 2);
    |                       ^^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: N >= 2', <path>/rstar/src/point.rs:313:23
    |
    = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info)

The error message is not terrible, but the code is hacky and would put an extra trait into the docs.

[michaelkirk]

That trait AssertGt1 error happens at compile time right? That's pretty cool, but I agree it's a little harder to follow.

I agree that the straight-forward runtime panic for this rare case is preferable.

Maybe once we get these two RFC's stabilized we could do something cleaner:

1. panic in const fn
2. const fn in trait (maybe not the best link... historical rustlang RFC discussions are sometimes hard for me to navigate)

[dominikWin]

Yes that error is at compile time.

[michaelkirk]

This LGTM, but I'll leave it open a couple days in case someone else wants to weigh in.

[michaelkirk]

bors r+

[bors]

Build failed:

• ci result

[michaelkirk]

Looks like we'll need to bump our MSRV (which I think is just fine - currently it's very old). @dominikWin - can you verify what minimum version of rust is required to build this branch?

[dominikWin]

1.55 is when array_map is stabilized, but 1.59 is when the project will compile since a few dependencies (rayon, which we use in criterion) want that version now.

[adamreichold]

1.55 is when array_map is stabilized, but 1.59 is when the project will compile since a few dependencies (rayon, which we use in criterion) want that version now.

We should still aim for 1.55 then and use cargo update --precise to pin compatible versions of our dependencies in the CI. (People building using the MSRV by definition do not have the latest toolchain and hence they are usually fine by not having the latest version of the dependencies as long as they are compatible.)

[adamreichold]

It appears that we have effectively bumped our MSRV to 1.63 at this point.

@dominikWin Could you rebase this? Thanks!

[dominikWin]

What moves rstar to rely on 1.63? I can't seem to find anything mentioned elsewhere.

[adamreichold]

What moves rstar to rely on 1.63? I can't seem to find anything mentioned elsewhere.

The changes to GitHub workflow, i.e. f5f0cb1, which tests our MSRV. But indeed, there is no changelog entry and the rust-version property also does not match. Will try to fix that separately.

EDIT: #124 is that separate PR.

[dominikWin]

bors try

[bors]

🔒 Permission denied

Existing reviewers: click here to make dominikWin a reviewer

[adamreichold]

bors try

bors ATM does not check anything that the CI itself does, so if you rebased and the CI is happen, you're golden.

I think the changelog entry is misplaced though? It still refers to the already released 0.10 section instead of the unreleased section.

[dominikWin]

I think the changelog entry is misplaced though? It still refers to the already released 0.10 section instead of the unreleased section.

Whoops. Should be fixed now

[adamreichold]

bors r=michaelkirk

[bors]

Build succeeded:

• ci result