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

Add more const fn functions #4

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

lrazovic
Copy link

@lrazovic lrazovic commented Sep 9, 2021

This PR aims to convert all the possibles functions to const fn in order to be used in other const fn environments, but also includes several changes to cleanup the code. I have also updated assert_matches to the latest version and replaced the deprecated merge_imports with imports_granularity in the rustfmt.toml.

As a downside, these changes bumps the MSRV to the 1.46.0.

@svartalf
Copy link
Owner

svartalf commented Sep 9, 2021

Looks great, thank you!

I see that CI fails, because it is not compatible with current MSRV policy (rustc 1.31+), so it will require a new major version release, which is I'm definitely up to (would be nice to include #3 too then).
Do we now what new minimal rustc version would be required now?

@lrazovic
Copy link
Author

The features #![feature(const_if_match)] and #![feature(const_loop)] were stabilized in rustc 1.46.0 (04488afe3 2020-08-24). [1, 2, 3]. The MSRV policy then will have to change to rustc 1.46+.

I'm going to read the discussion in #3 and I will do what I can to implement it.
Perhaps this approach would help? What do you think? serde-rs/serde#2001

@svartalf
Copy link
Owner

The MSRV policy then will have to change to rustc 1.46+.

Good to know, thank you. I'm totally okay with bumping it as long as we understand to what exactly it should be bumped :)

and I will do what I can to implement it.

I'm sorry, I didn't meant that you "should" do it, I was just pointing out that it would be great to include that PR into next release too. Nevertheless, tweaking serde implementation should be pretty straightforward, but it is still unclear to me what to do with fmt::Display implementation, as mentioned in #3. If you have any opinion on it, I would love to hear it

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

Successfully merging this pull request may close these issues.

None yet

2 participants