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

Doesn't compile on latest nightly #37

Closed
yotamofek opened this issue Jun 5, 2020 · 10 comments
Closed

Doesn't compile on latest nightly #37

yotamofek opened this issue Jun 5, 2020 · 10 comments

Comments

@yotamofek
Copy link

This PR broke the current functionality: rust-lang/rust#70107
Starting with the June 3rd nightly, staticvec fails to build.
IIUC, this is not a compiler bug, but rather a feature.
I think that until const bounds are implemented, there is no way to implement concat and all the other "combination" helpers.

error: constant expression depends on a generic parameter
    --> /Users/yotam/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/staticvec-0.9.3/src/lib.rs:1467:70
     |
1467 |   pub fn concat<const N2: usize>(&self, other: &StaticVec<T, N2>) -> StaticVec<T, { N + N2 }>
     |                                                                      ^^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: this may fail depending on what value the parameter takes
@pventuzelo
Copy link

+1

@slightlyoutofphase
Copy link
Owner

slightlyoutofphase commented Jun 13, 2020

Sorry guys, was busy with other stuff for a bit. Just noticed the automated tests were failing.

I'm going to attempt to find a workaround for this, as it seems probably not okay for me to just entirely remove all of the combination helpers for some unspecified amount of time considering they've been implemented in their current form and working perfectly for months at this point.

For the record I don't think I necessarily agree with this now being a compiler "error" in general, also. It's essentially saying, "simple compile-time addition of two must-be-known-at-compile-time-anyways usizes may fail", which while I guess is technically a possibility in theory would still indeed be caught at compile time and as such is not something that could lead to any kind of un-Rusty hard-to-notice runtime bugs.

I don't see who exactly in what exact scenario that PR is supposed to benefit.

@yotamofek
Copy link
Author

I'm not a rust contributor or anything,
but while on the one hand it kinda sucks because its a feature that's been working for months,
on the other hand - const generics are REALLY unstable and don't seem to be going towards stabilization anytime soon.

IIUC, this will be fixed somehow before the feature is stabilized, by allowing to specify bounds on const generics. Honestly, as a user of this library, I would rather get a "combinator"-less version of staticvec that can work with the latest nightly, rather than being stuck on an older nightly.. but that's just me. Not to sound ungrateful, because I'm definitely grateful for the work you put into this library. :)

@slightlyoutofphase
Copy link
Owner

slightlyoutofphase commented Jun 13, 2020

Oh yeah, I for sure want to get it working ASAP. Like, today ASAP. I'm just legitimately not sure what the "appropriate" thing to do here is if I cannot find a workaround.

For example, I could easily just remove the combination helper stuff entirely for now and release a 0.9.4 version of the crate without them, but that (I think) would be breaking the standard Rust versioning practices that people expect.

Maybe they'd be okay with it in this scenario, though. Or maybe not. It's hard to gauge. Any advice or input would be welcome!

@yotamofek
Copy link
Author

I do think it warrants a minor-version bump (0.10.0, I guess). That would also mean that any downstream crates would have to release a new version themselves with the updated dependency on staticvec to be able to compile on the latest nightly.
But it sounds to me like an alright-enough option, since it pretty much allows both people on a "pinned" nightly and people who prefer to always stay with the latest one to both compile with this lib. And it also won't break any downstream crates depending on the older version and the combination stuff (I mean, they will break on latest nightly no-matter-what, but if you were to release a 0.9.4 they would also break on nightly versions that would otherwise work).

That's just my own $0.02, I definitely am not an expert on any of the things I talked about, so take with grain of salt etc... :P

@slightlyoutofphase
Copy link
Owner

0.10.0 it is then I think. Will try to get it uploaded soon.

@slightlyoutofphase
Copy link
Owner

slightlyoutofphase commented Jun 13, 2020

Argh, now I have to find something different that looks cool to put in the readme also... this is the most annoying thing ever. No choice though...

@slightlyoutofphase
Copy link
Owner

I've put 0.10.0 up on crates.io now. Let me know if it fixes everything for your use case!

@yotamofek
Copy link
Author

Hey, I'm sorry for not being explicit enough sooner:
this definitely solved everything for my use case, closing the issue since you've solved it.
Thanks again!

@slightlyoutofphase
Copy link
Owner

No problem!

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

No branches or pull requests

3 participants