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

Error if is not allowed in a const fn when compiling with nightly #281

Closed
dunnock opened this issue Sep 29, 2020 · 10 comments
Closed

Error if is not allowed in a const fn when compiling with nightly #281

dunnock opened this issue Sep 29, 2020 · 10 comments
Labels
C-upstream Category: nothing actionable in the time crate

Comments

@dunnock
Copy link

dunnock commented Sep 29, 2020

Hello, by some reason I started getting compile errors on nigthly with time 0.2.22:

error[E0658]: `if` is not allowed in a `const fn`
  --> /usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/time-0.2.22/src/date.rs:36:5
   |
36 | /     if (remainder > 0 && b < 0) || (remainder < 0 && b > 0) {
37 | |         quotient - 1
38 | |     } else {
39 | |         quotient
40 | |     }
   | |_____^
   |
   = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
   = help: add `#![feature(const_if_match)]` to the crate attributes to enable

error[E0658]: `if` is not allowed in a `const fn`
   --> /usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/time-0.2.22/src/lib.rs:238:9
    |
238 | /         if $value < $start || $value > $end {
239 | |             return Err(crate::error::ComponentRange {
240 | |                 name: stringify!($value),
241 | |                 minimum: $start as i64,
...   |
247 | |             });
248 | |         }
    | |_________^
    | 
   ::: /usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/time-0.2.22/src/date.rs:136:9
    |
136 |           ensure_value_in_range!(year in MIN_YEAR => MAX_YEAR);
    |           ----------------------------------------------------- in this macro invocation
    |
    = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0658]: `if` is not allowed in a `const fn`
   --> /usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/time-0.2.22/src/lib.rs:238:9
    |
238 | /         if $value < $start || $value > $end {
239 | |             return Err(crate::error::ComponentRange {
240 | |                 name: stringify!($value),
241 | |                 minimum: $start as i64,
...   |
247 | |             });
248 | |         }
    | |_________^
    | 
   ::: /usr/local/cargo/registry/src/git.luolix.top-1ecc6299db9ec823/time-0.2.22/src/date.rs:137:9
    |
137 |           ensure_value_in_range!(month in 1 => 12);
    |           ----------------------------------------- in this macro invocation
    |
    = note: see issue #49146 <https://github.com/rust-lang/rust/issues/49146> for more information
    = help: add `#![feature(const_if_match)]` to the crate attributes to enable
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

was not able to dug the root cause, but might it be related with latest changes?

I am using nightly-2020-06-10

@dunnock
Copy link
Author

dunnock commented Sep 29, 2020

It seems time 0.2. was imported via actix-web v3.0.2 as a downstream dep, whilst time 0.1 was used accross the board. As I got 0.2.22 requires latest nightly, so closing this issue.

@dunnock dunnock closed this as completed Sep 29, 2020
@jhpratt
Copy link
Member

jhpratt commented Sep 29, 2020

This is caused by an upstream issue I noted about a month ago — taiki-e/const_fn#17.

Pinging @taiki-e now that there is a confirmed case of code breaking in the wild. Treating nightly as the then-beta compiler, rather than encompassing the entirety of the feature set that will be stabilized at some point during that release cycle, would avoid an issue like this.

It would be difficult to keep track of what is stabilized when (the exact date). The only alternative for me is to change every instance of 1.46 to 1.47, but I'd then be needlessly excluding anyone using a compiler that is fully capable of doing what is asked.

Frankly, if this behavior isn't changed, I'll consider forking. I'd rather fork than have time's support for nightly be like Swiss cheese.

@jhpratt jhpratt added the C-upstream Category: nothing actionable in the time crate label Sep 29, 2020
@dunnock
Copy link
Author

dunnock commented Sep 29, 2020

Thanks for details, good to know. FWIW I have fixed that issue downgrading to actix-web 2.0, going to first upgrade my nightly then will switch over.

@taiki-e
Copy link

taiki-e commented Sep 29, 2020

@jhpratt
IIUC, the current behavior of const_fn is the same as the behavior of time's build.rs when processing the nightly version. (Treating "1.N" as true on 1.N nightly)

I'm ok if change the implementation to treat const_fn("1.N") as false on 1.N-nightly (I don't have a very strong opinion), but I've never actually seen a crate using a build script based on that logic. (In any case, once cfg-version is stable, I'll change behavior to follow its specifications.)

Frankly, if this behavior isn't changed, I'll consider forking. I'd rather fork than have time's support for nightly be like Swiss cheese.

FWIW, const_fn accepts cfg as an argument. Given that the current time's build script probably has the same problem, it might be good to manage everything by time's build script.

@jhpratt
Copy link
Member

jhpratt commented Sep 29, 2020

The time crate relies on version_check. A quick check of version_check's behavior actually shows that time's current handling is wrong, and I will fix this.

The standback crate doesn't follow that exact behavior, but that's intentional — being able to push a standback release before a Rust release is important for that. It would be trivial to swap over the handling to match the RFC.

FWIW, const_fn accepts cfg as an argument. Given that the current time's build script probably has the same problem, it might be good to manage everything by time's build script.

As I said above, I'll fix time's behavior, as I consider that to be a bug. I'd obviously prefer to avoid forking, and would also prefer to not emit a cfg flag for every version (as is what is done with standback).

@taiki-e
Copy link

taiki-e commented Sep 29, 2020

would also prefer to not emit a cfg flag for every version

You don't have to emit cfg for all versions. It's probably enough to add code here that emits cfg flag if 1.N+ and replace the existing const_fn("1.N") with const_fn(cfg(flag_name)).

@taiki-e
Copy link

taiki-e commented Sep 29, 2020

You don't have to emit cfg for all versions. It's probably enough to add code here that emits cfg flag if 1.N+ and replace the existing const_fn("1.N") with const_fn(cfg(flag_name)).

I'll revisit the current behavior in taiki-e/const_fn#27, but it's probably not immediately decided, so I recommend using const_fn(cfg(...)) at this time. (If you need it, I'll happy to write a patch for it.)

@jhpratt
Copy link
Member

jhpratt commented Sep 29, 2020

Using const_fn(cfg(…)) is less readable (and as such less maintainable) than simply declaring the version number right next to where it's used. The current usage of cfg flags in build.rs are for runtime features that can't be omitted unless using the rustversion crate, which unfortunately relies on some hefty proc macro crates.

@taiki-e
Copy link

taiki-e commented Oct 30, 2020

The current usage of cfg flags in build.rs are for runtime features that can't be omitted unless using the rustversion crate, which unfortunately relies on some hefty proc macro crates.

FWIW, rustversion crate recently dropped all dependencies: dtolnay/rustversion#24

@jhpratt
Copy link
Member

jhpratt commented Oct 30, 2020

Good to know! I'll take a look at what can be converted to that crate, as it's certainly more powerful than the const_fn crate (it looks like a feature superset).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-upstream Category: nothing actionable in the time crate
Projects
None yet
Development

No branches or pull requests

3 participants