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

Support for version-based code generation #17

Merged
merged 1 commit into from
Aug 25, 2020
Merged

Support for version-based code generation #17

merged 1 commit into from
Aug 25, 2020

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Aug 24, 2020

This rewrites the const_fn attribute to support the following four syntaxes:

use const_fn::const_fn;

// 1.36 and later compiler (including beta and nightly)
#[const_fn("1.36")]
pub const fn version() {
    /* ... */
}

// nightly compiler (including dev build)
#[const_fn(nightly)]
pub const fn nightly() {
    /* ... */
}

// `cfg(...)`
#[const_fn(cfg(...))]
pub const fn cfg() {
    /* ... */
}

// `cfg(feature = "...")`
#[const_fn(feature = "...")]
pub const fn feature() {
    /* ... */
}

This does not provide any special support for beta. This is because unlike nighty there are no unstable features (features available in beta are already stabilized, so I think it is preferable to specify that version).

Closes #15

@taiki-e taiki-e added breaking-change This proposes a breaking change C-enhancement Category: A new feature or an improvement for an existing one and removed C-enhancement Category: A new feature or an improvement for an existing one labels Aug 24, 2020
@jhpratt
Copy link

jhpratt commented Aug 24, 2020

Just for reference, when cfg_version is implemented (if it's not already), the lsng team decided it was best to have nightly be treated as one version prior — essentially the current beta. This is because there's no guarantee that a feature available in a given nightly will land two releases later.

I can pull up a reference tomorrow if you'd like, as I'm on mobile right now. I believe I'm remembering it correctly, though.

Also, may I suggest you check out the version_check crate? It's widely used and does pretty much the same thing as you're doing in the build script.

@taiki-e
Copy link
Owner Author

taiki-e commented Aug 24, 2020

Just for reference, when cfg_version is implemented (if it's not already), the lsng team decided it was best to have nightly be treated as one version prior — essentially the current beta. This is because there's no guarantee that a feature available in a given nightly will land two releases later.

Hmm, I think that discussion has not been finished yet: rust-lang/rust#64796 (comment)

Also, may I suggest you check out the version_check crate? It's widely used and does pretty much the same thing as you're doing in the build script.

Sounds good.

@taiki-e taiki-e force-pushed the version branch 2 times, most recently from 4d47247 to 9a416d2 Compare August 24, 2020 09:15
@taiki-e taiki-e mentioned this pull request Aug 24, 2020
@taiki-e taiki-e changed the title [WIP] Support for version-based code generation Support for version-based code generation Aug 24, 2020
Comment on lines +35 to +38
// 1.36 and later compiler (including beta and nightly)
#[const_fn("1.36")]
pub const fn version() {
/* ... */
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It currently only accepts string literals as version requirements, like cfg_version does.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #19

Comment on lines +41 to +44
// nightly compiler (including dev build)
#[const_fn(nightly)]
pub const fn nightly() {
/* ... */
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use the nightly only unstable feature, it needs to enable the feature at crate-level, and in any case, it needs build-script. So, this may not be very useful in practice, but I'll keep it for now.

@jhpratt
Copy link

jhpratt commented Aug 24, 2020

Hmm, I think that discussion has not been finished yet: rust-lang/rust#64796 (comment)

Looks like I must've missed that comment; I just remembered this comment off the top of my head. Changing the behavior would of course be a breaking change, so it's something to be mindful of at the least.


Is it common/accepted practice to write details to a file, which is later accessed as part of the compilation process? It's a neat trick, but I don't think I've ever seen it anywhere before. I've always used rustc cfgs, prefixed with something unique to the crate.

@taiki-e
Copy link
Owner Author

taiki-e commented Aug 25, 2020

Is it common/accepted practice to write details to a file, which is later accessed as part of the compilation process? It's a neat trick, but I don't think I've ever seen it anywhere before. I've always used rustc cfgs, prefixed with something unique to the crate.

cfgs have no values, so version info should be output another way. (Of course, we can also output a cfg like const_fn_version_is_1_36_0, but we'll need a lot of const or if statements to convert it to version info)

I don't know how widely it is used, but at least I think bindgen users are using it.

@taiki-e
Copy link
Owner Author

taiki-e commented Aug 25, 2020

bors r+

There may be minor bugs and lack of documentation, but I think the implementation is ok with this.
After merging this and #18, I plan to make a new release.

@bors
Copy link
Contributor

bors bot commented Aug 25, 2020

Build succeeded:

@bors bors bot merged commit 7d29667 into master Aug 25, 2020
@bors bors bot deleted the version branch August 25, 2020 02:43
@taiki-e taiki-e mentioned this pull request Aug 25, 2020
bors bot added a commit that referenced this pull request Aug 25, 2020
21: Release 0.4.0 r=taiki-e a=taiki-e

Changes:

* [Add support for version-based code generation.](#17) The following conditions are available:

  ```rust
  use const_fn::const_fn;

  // function is `const` on specified version and later compiler (including beta and nightly)
  #[const_fn("1.36")]
  pub const fn version() {
      /* ... */
  }

  // function is `const` on nightly compiler (including dev build)
  #[const_fn(nightly)]
  pub const fn nightly() {
      /* ... */
  }

  // function is `const` if `cfg(...)` is true
  #[const_fn(cfg(...))]
  pub const fn cfg() {
      /* ... */
  }

  // function is `const` if `cfg(feature = "...")` is true
  #[const_fn(feature = "...")]
  pub const fn feature() {
      /* ... */
  }
  ```

* Improve compile time by removing proc-macro related dependencies ([#18](#18), [#20](#20)).

Co-authored-by: Taiki Endo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This proposes a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate code based on version rather than cfg
2 participants