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

Stabilize underscore_const_names in 1.37.0 #61347

Merged
merged 3 commits into from
Jun 16, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented May 30, 2019

You are now permitted to write:

const _: $type_expression = $term_expression;

That is, we change the grammar of items, as written in the .lyg notation, from:

Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IDENT ":" ty:Type "=" value:Expr ";" }
  | ...
  ;

into:

Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IdentOrUnderscore ":" ty:Type "=" value:Expr ";" }
  | ...
  ;

IdentOrUnderscore =
  | Named:IDENT
  | NoName:"_"
  ;

r? @petrochenkov

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. relnotes Marks issues that should be documented in the release notes of the next release. labels May 30, 2019
@Centril Centril added this to the 1.37 milestone May 30, 2019
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 30, 2019
@Centril Centril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 30, 2019
@Centril Centril changed the title Stabilize underscore_const_names in 1.37.0 [WIP] Stabilize underscore_const_names in 1.37.0 May 30, 2019
@Centril Centril force-pushed the stabilize-underscore_const_names branch from e998227 to 85b1489 Compare May 30, 2019 14:20
@rust-highfive

This comment has been minimized.

@Centril Centril force-pushed the stabilize-underscore_const_names branch from 85b1489 to 63829d2 Compare May 30, 2019 16:34
@Centril Centril changed the title [WIP] Stabilize underscore_const_names in 1.37.0 Stabilize underscore_const_names in 1.37.0 May 31, 2019
@Centril Centril added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. I-nominated and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 31, 2019
@Centril
Copy link
Contributor Author

Centril commented May 31, 2019

Stabilization proposal

I propose that we stabilize #![feature(underscore_const_names)].

@rfcbot merge

Tracking issue: #54912
Version target: 1.37 (2019-07-04 => beta, 2019-08-15 => stable).

What is stabilized

You are now permitted to write:

const _: $type_expression = $term_expression;

That is, we change the grammar of items, as written in the .lyg notation, from:

Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IDENT ":" ty:Type "=" value:Expr ";" }
  | ...
  ;

into:

Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IdentOrUnderscore ":" ty:Type "=" value:Expr ";" }
  | ...
  ;

IdentOrUnderscore =
  | Named:IDENT
  | NoName:"_"
  ;

When name = NoName occurs then there are no run-time semantics because an unnamed constant cannot be referred to. With respect to compile-time dynamic semantics, the same rules apply as an unused constant has today. E.g., if you have const _: () = <diverges>; then #[deny(const_err)] will trigger. As for static semantics, an unnamed constant item behaves as normal with respect to type checking.

What is not stabilized

  • Only a free const item may use _ as its name. Associated ones may not.
  • Non-const items may not use _ as a name. Notably, static items may not.
  • const items may not use arbitrary irrefutable patterns. E.g., we do not permit const (A, B): (u8, u8) = (1, 2);

Divergences from RFC and unresolved questions

None.

Tests

The tests can be primarily seen in the PR itself. Here are some of them:

History

Motivation

The general motivation for const _: $type = $expr; is to facilitate writing well-behaved macros.
For example, procedural macros such as serde_derive generate a constant item to wrap the implementation into. Another example of this is proptest_derive which generates roughly:

#[allow(non_upper_case_globals)]
const _IMPL_ARBITRARY_FOR_MyType: () = {
    impl proptest::arbitrary::Arbitrary for MyType {
        ...
    }
};

Instead, it would be better to generate:

const _: () = {
    impl proptest::arbitrary::Arbitrary for MyType {
        ...
    }
};

This not only has the advantage of generating less code, but also produces better diagnostics.

Other motivational examples include:

More generally, const _: () = $expr; is an idiom allowing users to type check a piece of code but not necessarily more than that.

Potential concerns

There is some concern that underscore_const_names is an ad-hoc way to achieve the desired outcomes. In particular, instead of supporting identifiers or an underscore, we could support arbitrary irrefutable patterns for const items. Moreover, stabilizing const _ would further diverge constant items from static items which could ostensibly also support underscores or even arbitrary irrefutable patterns. However, there are open semantic questions with static _ and arbitrary irrefutable patterns for const requires more work in the compiler. In any case, the stabilization of const _ still keeps us forward compatible with such extensions and therefore these concerns may be put aside for the time being.

There is also some concern of how const _ would compose with associated constants as well as generic ones. Presumably, something like const (A, _)<T>... does not make sense but we also do not have to support the combination of arbitrary irrefutable patterns and generic constants. As for associated constants, one could imagine defining or providing multiple associated constants through pattern matching one constant expression inside an implementation. All in all, the generalization problems do not seem insurmountable.

Some considered alternatives

Unnamed modules, mod _ { ... }

As an alternative to const _: $type = $expr; one could allow mod _ { ... } instead.
The grammar for mod items would use the same IdentOrUnderscore production. When considering mod _ { ... } note that:

  • It is often the point of const _: $type = $expr; to check that an $expression satisfies some condition, e.g., that it is an expression that does not diverge (const_assert!(...)). In these cases, mod _ { ... } is at a disadvantage because the body accepts items and not expressions. To remedy this, you'd need a const item inside the mod item thus unnecessarily generating more tokens. Meanwhile, fitting an item inside an expr requires less.

  • mod _ { ... } has the same problems wrt. generics as const _: $type = $expr; has. Namely, one could consider generic modules mod _<T, ...> { ... }. However, there's no strange interactions between pattern matching and generics here.

  • mod _ { ... } has fewer interactions and is less generalizable. This can both be a good thing because it is more "complete" sooner but it can also feel more ad-hoc. However, we already have use foo as _; so the production IndentOrUnderscore already exists in stable Rust.

Overall, we believe const _ to be a better fit because it has a natural generalization and works better with macros overall.

const { ... } blocks

Another alternative would be some sort of const { ... } block. This would take the grammatical form:

ItemKind =
  | ...
  | ConstBlock:{ "const" value:Block }
  | ...
  ;

ExprKind =
  | ...
  | ConstBlock:{ "const" value:Block }
  | ...
  ;

Notably, const { ... } would have the double duty as an expression and item form.
Assigning unsurprising semantics to const { ... } as an expression is easy: the block is a const context and as any block it may contain a series of statements followed by an optional tail expression which is the value of the block. As an item, it is less clear what the tail expression would be. Would we require that the tail expression be implicitly coercible to ()? Would we allow a tail expression at all? Overall, a const { ... } is probably a better fit solely as an expression form and not as an item.

Possible future work

As aforementioned, natural extensions to const _: $type = $expr; primarily includes:

  • Extending _ to $pat.
  • Extending _ or $pat to static items as well. In this case we should figure out what consistent, useful, and expected dynamic semantics are for situations such as static (A, _): (u8, u8) = (1, 2);.
  • Finally, we should consider whether it would make sense to extend const _ to associated constants as well and what implications this has for static items.

@rfcbot
Copy link

rfcbot commented May 31, 2019

Team member @Centril has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels May 31, 2019
@Centril
Copy link
Contributor Author

Centril commented May 31, 2019

N.B. Aaron is on leave so I've checked his box.

@petrochenkov
Copy link
Contributor

Implementation LGMT, marking as waiting on team.

@petrochenkov petrochenkov added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 31, 2019
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jun 6, 2019
@rfcbot
Copy link

rfcbot commented Jun 6, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@bors

This comment has been minimized.

@Centril Centril added S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Jun 13, 2019
@brson
Copy link
Contributor

brson commented Jun 14, 2019

Looks like quite a hack to achieve the stated outcome. Reminds me of javascript.

@rfcbot
Copy link

rfcbot commented Jun 16, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 16, 2019
@Centril
Copy link
Contributor Author

Centril commented Jun 16, 2019

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Jun 16, 2019

📌 Commit e62c9d7 has been approved by petrochenkov

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 16, 2019
@Centril Centril removed the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Jun 16, 2019
@bors
Copy link
Contributor

bors commented Jun 16, 2019

⌛ Testing commit e62c9d7 with merge 4edff84...

bors added a commit that referenced this pull request Jun 16, 2019
…trochenkov

Stabilize underscore_const_names in 1.37.0

You are now permitted to write:

```rust
const _: $type_expression = $term_expression;
```

That is, we change the [grammar of items](https://github.com/rust-lang-nursery/wg-grammar/blob/9d1984d7ae8d6576f943566539a31a5800644c57/grammar/item.lyg#L3-L42), as written in [the *`.lyg`* notation](https://github.com/rust-lang/gll/tree/263bf161dad903e67aa65fc591ced3cab18afa2a#grammar), from:

```java
Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IDENT ":" ty:Type "=" value:Expr ";" }
  | ...
  ;
```

into:

```java
Item = attrs:OuterAttr* vis:Vis? kind:ItemKind;
ItemKind =
  | ...
  | Const:{ "const" name:IdentOrUnderscore ":" ty:Type "=" value:Expr ";" }
  | ...
  ;

IdentOrUnderscore =
  | Named:IDENT
  | NoName:"_"
  ;
```

r? @petrochenkov
@bors
Copy link
Contributor

bors commented Jun 16, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing 4edff84 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 16, 2019
@bors bors merged commit e62c9d7 into rust-lang:master Jun 16, 2019
@Centril Centril deleted the stabilize-underscore_const_names branch June 16, 2019 23:26
@nvzqz
Copy link
Contributor

nvzqz commented Aug 12, 2019

Thanks @Centril for putting in the work in getting this feature stabilized. With it, I should be able to release 1.0 of static_assertions with the release of 1.37.0 this Thursday! 🎉

@Centril
Copy link
Contributor Author

Centril commented Aug 13, 2019

@nvzqz Happy to see this already having a nice impact! :)

Maybe we can start using static_assertions in the compiler as well? -- cc @Zoxc

@eddyb
Copy link
Member

eddyb commented Aug 14, 2019

I think we have our own hacky version in rustc_data_structures?
Besides, const _: () = assert!(...); is the static_assert! we want 😉

@Centril
Copy link
Contributor Author

Centril commented Aug 14, 2019

@nvzqz
Copy link
Contributor

nvzqz commented Aug 14, 2019

The crate offers functionality outside of const_assert! that the compiler could also leverage. Not sure where exactly since I'm not sufficiently familiar with its internals.

@eddyb I hope to make const_assert! wrap around assert! like that someday. It would produce significantly better error messages.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 29, 2019
Pkgsrc changes:
 * Add a patch to llvm to deal with const dli_saddr.
 * Adapt two other patches.
 * Cross-build currently fails, so i386, powerpc and sparc64 bootstrap
   kits for 1.37.0 are built natively.  Missing aarch64 hardware, so that's
   not available yet.
 * Bump bootstrap requirements to 1.36.0 except for armv7-unknown-netbsd-eabihf
   which I've not managed to cross-build.

Upstream changes:

Version 1.37.0 (2019-08-15)
==========================

Language
--------
- `#[must_use]` will now warn if the type is contained in a [tuple][61100],
  [`Box`][62228], or an [array][62235] and unused.
- [You can now use the `cfg` and `cfg_attr` attributes on
  generic parameters.][61547]
- [You can now use enum variants through type alias.][61682] e.g. You can
  write the following:
  ```rust
  type MyOption = Option<u8>;

  fn increment_or_zero(x: MyOption) -> u8 {
      match x {
          MyOption::Some(y) => y + 1,
          MyOption::None => 0,
      }
  }
  ```
- [You can now use `_` as an identifier for consts.][61347] e.g. You can write
  `const _: u32 = 5;`.
- [You can now use `#[repr(align(X)]` on enums.][61229]
- [The  `?`/_"Kleene"_ macro operator is now available in the
  2015 edition.][60932]

Compiler
--------
- [You can now enable Profile-Guided Optimization with the `-C profile-generate`
  and `-C profile-use` flags.][61268] For more information on how to use profile
  guided optimization, please refer to the [rustc book][rustc-book-pgo].
- [The `rust-lldb` wrapper script should now work again.][61827]

Libraries
---------
- [`mem::MaybeUninit<T>` is now ABI-compatible with `T`.][61802]

Stabilized APIs
---------------
- [`BufReader::buffer`]
- [`BufWriter::buffer`]
- [`Cell::from_mut`]
- [`Cell<[T]>::as_slice_of_cells`][`Cell<slice>::as_slice_of_cells`]
- [`DoubleEndedIterator::nth_back`]
- [`Option::xor`]
- [`Wrapping::reverse_bits`]
- [`i128::reverse_bits`]
- [`i16::reverse_bits`]
- [`i32::reverse_bits`]
- [`i64::reverse_bits`]
- [`i8::reverse_bits`]
- [`isize::reverse_bits`]
- [`slice::copy_within`]
- [`u128::reverse_bits`]
- [`u16::reverse_bits`]
- [`u32::reverse_bits`]
- [`u64::reverse_bits`]
- [`u8::reverse_bits`]
- [`usize::reverse_bits`]

Cargo
-----
- [`Cargo.lock` files are now included by default when publishing executable crates
  with executables.][cargo/7026]
- [You can now specify `default-run="foo"` in `[package]` to specify the
  default executable to use for `cargo run`.][cargo/7056]

Misc
----

Compatibility Notes
-------------------
- [Using `...` for inclusive range patterns will now warn by default.][61342]
  Please transition your code to using the `..=` syntax for inclusive
  ranges instead.
- [Using a trait object without the `dyn` will now warn by default.][61203]
  Please transition your code to use `dyn Trait` for trait objects instead.

[62228]: rust-lang/rust#62228
[62235]: rust-lang/rust#62235
[61802]: rust-lang/rust#61802
[61827]: rust-lang/rust#61827
[61547]: rust-lang/rust#61547
[61682]: rust-lang/rust#61682
[61268]: rust-lang/rust#61268
[61342]: rust-lang/rust#61342
[61347]: rust-lang/rust#61347
[61100]: rust-lang/rust#61100
[61203]: rust-lang/rust#61203
[61229]: rust-lang/rust#61229
[60932]: rust-lang/rust#60932
[cargo/7026]: rust-lang/cargo#7026
[cargo/7056]: rust-lang/cargo#7056
[`BufReader::buffer`]: https://doc.rust-lang.org/std/io/struct.BufReader.html#method.buffer
[`BufWriter::buffer`]: https://doc.rust-lang.org/std/io/struct.BufWriter.html#method.buffer
[`Cell::from_mut`]: https://doc.rust-lang.org/std/cell/struct.Cell.html#method.from_mut
[`Cell<slice>::as_slice_of_cells`]: https://doc.rust-lang.org/std/cell/struct.Cell.html#method.as_slice_of_cells
[`DoubleEndedIterator::nth_back`]: https://doc.rust-lang.org/std/iter/trait.DoubleEndedIterator.html#method.nth_back
[`Option::xor`]: https://doc.rust-lang.org/std/option/enum.Option.html#method.xor
[`RefCell::try_borrow_unguarded`]: https://doc.rust-lang.org/std/cell/struct.RefCell.html#method.try_borrow_unguarded
[`Wrapping::reverse_bits`]: https://doc.rust-lang.org/std/num/struct.Wrapping.html#method.reverse_bits
[`i128::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i128.html#method.reverse_bits
[`i16::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i16.html#method.reverse_bits
[`i32::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i32.html#method.reverse_bits
[`i64::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i64.html#method.reverse_bits
[`i8::reverse_bits`]: https://doc.rust-lang.org/std/primitive.i8.html#method.reverse_bits
[`isize::reverse_bits`]: https://doc.rust-lang.org/std/primitive.isize.html#method.reverse_bits
[`slice::copy_within`]: https://doc.rust-lang.org/std/primitive.slice.html#method.copy_within
[`u128::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u128.html#method.reverse_bits
[`u16::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u16.html#method.reverse_bits
[`u32::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u32.html#method.reverse_bits
[`u64::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u64.html#method.reverse_bits
[`u8::reverse_bits`]: https://doc.rust-lang.org/std/primitive.u8.html#method.reverse_bits
[`usize::reverse_bits`]: https://doc.rust-lang.org/std/primitive.usize.html#method.reverse_bits
[rustc-book-pgo]: https://doc.rust-lang.org/rustc/profile-guided-optimization.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants