Repository: JelteF/derive_more
Pull request #218 (2022-11-10 11:04:46)

Add `const` to constructors by a feature
Resolves [Constructor generated new should be marked const](https://github.com/JelteF/derive_more/issues/131)

## Synopsis

Added a new `const` feature which generates `const fn` on `Constructor` derives.

## Solution

## Checklist

- [x] Documentation is updated
- [x] Tests are added/updated **IMPORTANT**: I am not sure if new tests are added to the CI.
- [x] [CHANGELOG entry][l:1] is added


tyranron: Should this really be feature-gated? I cannot imagine the situation where `const` will be harmful on constructors.
fmiguelgarcia: > Should this really be feature-gated? I cannot imagine the situation where `const` will be harmful on constructors.

Just in case you are using a legacy compiler with no support of `const` in Generics.
Having said that, I think `const` feature should be enable by default, WDYT?

att. @tyranron 
tyranron: @fmiguelgarcia well, once we release 1.0 (next release) we will have [MSRV set to 1.65](https://github.com/JelteF/derive_more/issues/202), so you won't be able to use legacy compilers 🤷‍♂️
JelteF: Also, I feel like the same should be done for the `IsVariant` and `Unwrap` derives.
tyranron: @JelteF `IsVariant` yes, but `Unwrap` definitely not, as `const` at the moment is not very clever with destructor evaluation checks, so almost any struct/enum destruction in `const` leads to compilation error. See https://github.com/rust-lang/rust/issues/60964 for details.
fmiguelgarcia: @tyranron I've used the `const` feature to enable const in `Unwrap` (I've just read your comment).
`IsVariant` and `Constructor` now uses `const fn`.

Do you want to remove `const` feature completely? Or Do you want to keep it and support `const` in `Unwrap`? 
att. @JelteF @tyranron 
 
tyranron: @fmiguelgarcia I'd vote for removing `const` feature completely. We always may `const`ify `Unwrap` once `feature(const_precise_live_drops)` is stabilized. It's not something that vital, deserving to squat-around it with an additional Cargo feature.
JelteF: I think this is almost good to merge, just a few things are needed:
1. Remove const from unwrap due to the bugs (so only const for constructor and isvariant for now)
2. Remove the const feature and simply always use const.
3. fix clippy errors.
JelteF: @fmiguelgarcia do you have time to do that? This is one of the final things left on the 1.0.0 milestone.

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index caac096..8ff52d0 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -64,16 +64,16 @@ jobs:
       - uses: actions/checkout@v3
       - uses: dtolnay/rust-toolchain@v1
         with:
-          toolchain: ${{ matrix.msrv }}
-      - run: rustup default ${{ matrix.msrv }}
+          toolchain: nightly
       - uses: dtolnay/rust-toolchain@v1
         with:
-          toolchain: nightly
+          toolchain: ${{ matrix.msrv }}
 
       - name: Install minimal dependencies versions
         run: cargo +nightly update -Z minimal-versions
 
       - run: cargo test --workspace --features full,testing-helpers
+                        -- --skip compile_fail
 
   no_std:
     runs-on: ubuntu-latest
@@ -111,7 +111,6 @@ jobs:
       - uses: dtolnay/rust-toolchain@v1
         with:
           toolchain: ${{ matrix.toolchain }}
-      - run: rustup default ${{ matrix.toolchain }}
 
       - run: cargo test --workspace --features testing-helpers
 
@@ -123,7 +122,6 @@ jobs:
       - uses: dtolnay/rust-toolchain@v1
         with:
           toolchain: nightly
-      - run: rustup default nightly
 
       - name: Install tomljson
         run: |
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 212e635..5eb6715 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Add `FromStr` derive support for enums that contain variants without fields.
   If you pass the name of the variant to `from_str` it will create the matching
   variant.
+- Use `const fn` in `Constructor` and `IsVariant` derives.
 
 ### Improvements
 
diff --git a/impl/doc/constructor.md b/impl/doc/constructor.md
index 1f19595..43f55c0 100644
--- a/impl/doc/constructor.md
+++ b/impl/doc/constructor.md
@@ -26,7 +26,7 @@ Code like this will be generated:
 ```rust
 # struct MyInts(i32, i32);
 impl MyInts {
-    pub fn new(__0: i32, __1: i32) -> MyInts {
+    pub const fn new(__0: i32, __1: i32) -> MyInts {
         MyInts(__0, __1)
     }
 }
@@ -60,7 +60,7 @@ Code like this will be generated:
 #     y: i32,
 # }
 impl Point2D {
-    pub fn new(x: i32, y: i32) -> Point2D {
+    pub const fn new(x: i32, y: i32) -> Point2D {
         Point2D { x: x, y: y }
     }
 }
diff --git a/impl/doc/is_variant.md b/impl/doc/is_variant.md
index 6109259..9e549dc 100644
--- a/impl/doc/is_variant.md
+++ b/impl/doc/is_variant.md
@@ -33,10 +33,10 @@ The derive in the above example code generates the following code:
 #     Nothing
 # }
 impl <T> Maybe<T>{
-    pub fn is_just(&self) -> bool {
+    pub const fn is_just(&self) -> bool {
         match self {Self::Just(..) => true, _ => false}
     }
-    pub fn is_nothing(&self) -> bool {
+    pub const fn is_nothing(&self) -> bool {
         match self {Self::Nothing => true, _ => false}
     }
 }
diff --git a/impl/src/constructor.rs b/impl/src/constructor.rs
index 6d8fb41..284b064 100644
--- a/impl/src/constructor.rs
+++ b/impl/src/constructor.rs
@@ -29,7 +29,7 @@ pub fn expand(input: &DeriveInput, _: &str) -> TokenStream {
         #[automatically_derived]
         impl #impl_generics #input_type #ty_generics #where_clause {
             #[inline]
-            pub fn new(#(#vars: #original_types),*) -> #input_type #ty_generics {
+            pub const fn new(#(#vars: #original_types),*) -> #input_type #ty_generics {
                 #body
             }
         }
diff --git a/impl/src/is_variant.rs b/impl/src/is_variant.rs
index 2d5e074..b6c074f 100644
--- a/impl/src/is_variant.rs
+++ b/impl/src/is_variant.rs
@@ -45,7 +45,7 @@ pub fn expand(input: &DeriveInput, trait_name: &'static str) -> Result<TokenStre
             #[doc = "Returns `true` if this value is of type `"]
             #[doc = #variant_name]
             #[doc = "`. Returns `false` otherwise"]
-            pub fn #fn_name(&self) -> bool {
+            pub const fn #fn_name(&self) -> bool {
                 match self {
                     #enum_name ::#variant_ident #data_pattern => true,
                     _ => false
diff --git a/impl/src/parsing.rs b/impl/src/parsing.rs
index 952b8e7..c465a04 100644
--- a/impl/src/parsing.rs
+++ b/impl/src/parsing.rs
@@ -412,7 +412,7 @@ fn identifier(input: &str) -> Option<(LeftToParse<'_>, Identifier<'_>)> {
 /// [1]: https://doc.rust-lang.org/stable/std/fmt/index.html#syntax
 fn integer(input: &str) -> Option<(LeftToParse<'_>, usize)> {
     and_then(
-        take_while1(check_char(|c| matches!(c, '0'..='9'))),
+        take_while1(check_char(|c| c.is_ascii_digit())),
         |(i, int)| int.parse().ok().map(|int| (i, int)),
     )(input)
 }


Test patch:
diff --git a/tests/constructor.rs b/tests/constructor.rs
index 4b9ff0d..8f9be45 100644
--- a/tests/constructor.rs
+++ b/tests/constructor.rs
@@ -5,17 +5,27 @@ use derive_more::Constructor;
 #[derive(Constructor)]
 struct EmptyTuple();
 
+const EMPTY_TUPLE: EmptyTuple = EmptyTuple::new();
+
 #[derive(Constructor)]
 struct EmptyStruct {}
 
+const EMPTY_STRUCT: EmptyStruct = EmptyStruct::new();
+
 #[derive(Constructor)]
 struct EmptyUnit;
 
+const EMPTY_UNIT: EmptyUnit = EmptyUnit::new();
+
 #[derive(Constructor)]
 struct MyInts(i32, i32);
 
+const MY_INTS: MyInts = MyInts::new(1, 2);
+
 #[derive(Constructor)]
 struct Point2D {
     x: i32,
     y: i32,
 }
+
+const POINT_2D: Point2D = Point2D::new(-4, 7);
diff --git a/tests/is_variant.rs b/tests/is_variant.rs
index 07543a8..d8f41ad 100644
--- a/tests/is_variant.rs
+++ b/tests/is_variant.rs
@@ -8,18 +8,59 @@ enum Either<TLeft, TRight> {
     Right(TRight),
 }
 
+const _: () = {
+    let either: Either<u8, i16> = Either::Right(7);
+    assert!(either.is_right());
+    assert!(!either.is_left());
+
+    let either: Either<u8, i16> = Either::Left(7);
+    assert!(!either.is_right());
+    assert!(either.is_left());
+};
+
 #[derive(IsVariant)]
 enum Maybe<T> {
     Nothing,
     Just(T),
 }
 
+const _: () = {
+    let maybe: Maybe<u8> = Maybe::Just(7);
+    assert!(maybe.is_just());
+    assert!(!maybe.is_nothing());
+
+    let maybe: Maybe<u8> = Maybe::Nothing;
+    assert!(!maybe.is_just());
+    assert!(maybe.is_nothing());
+};
+
+#[test]
+pub fn test_is_variant() {
+    assert!(Maybe::<()>::Nothing.is_nothing());
+    assert!(!Maybe::<()>::Nothing.is_just());
+}
+
 #[derive(IsVariant)]
 enum Color {
     RGB(u8, u8, u8),
     CMYK { c: u8, m: u8, y: u8, k: u8 },
 }
 
+const _: () = {
+    let color = Color::RGB(0, 0, 0);
+    assert!(color.is_rgb());
+    assert!(!color.is_cmyk());
+
+    let color = Color::CMYK {
+        c: 0,
+        m: 0,
+        y: 0,
+        k: 0,
+    };
+    assert!(!color.is_rgb());
+    assert!(color.is_cmyk());
+};
+
 #[derive(IsVariant)]
 enum Nonsense<'a, T> {
     Ref(&'a T),
@@ -28,6 +69,16 @@ enum Nonsense<'a, T> {
     NoRefIgnored,
 }
 
+const _: () = {
+    let nonsense: Nonsense<u8> = Nonsense::Ref(&7);
+    assert!(nonsense.is_ref());
+    assert!(!nonsense.is_no_ref());
+
+    let nonsense: Nonsense<u8> = Nonsense::NoRef;
+    assert!(!nonsense.is_ref());
+    assert!(nonsense.is_no_ref());
+};
+
 #[derive(IsVariant)]
 enum WithConstraints<T>
 where
@@ -36,6 +87,17 @@ where
     One(T),
     Two,
 }
+
+const _: () = {
+    let wc: WithConstraints<u8> = WithConstraints::One(1);
+    assert!(wc.is_one());
+    assert!(!wc.is_two());
+
+    let wc: WithConstraints<u8> = WithConstraints::Two;
+    assert!(!wc.is_one());
+    assert!(wc.is_two());
+};
+
 #[derive(IsVariant)]
 enum KitchenSink<'a, 'b, T1: Copy, T2: Clone>
 where
@@ -49,8 +111,52 @@ where
     NothingToSeeHere {},
 }
 
-#[test]
-pub fn test_is_variant() {
-    assert!(Maybe::<()>::Nothing.is_nothing());
-    assert!(!Maybe::<()>::Nothing.is_just());
-}
+const _: () = {
+    let ks: KitchenSink<u16, u8> = KitchenSink::Left(&1);
+    assert!(ks.is_left());
+    assert!(!ks.is_right());
+    assert!(!ks.is_own_both());
+    assert!(!ks.is_empty());
+    assert!(!ks.is_never_mind());
+    assert!(!ks.is_nothing_to_see_here());
+
+    let ks: KitchenSink<u16, u8> = KitchenSink::Right(&1);
+    assert!(!ks.is_left());
+    assert!(ks.is_right());
+    assert!(!ks.is_own_both());
+    assert!(!ks.is_empty());
+    assert!(!ks.is_never_mind());
+    assert!(!ks.is_nothing_to_see_here());
+
+    let ks: KitchenSink<u16, u8> = KitchenSink::OwnBoth { left: 1, right: 2 };
+    assert!(!ks.is_left());
+    assert!(!ks.is_right());
+    assert!(ks.is_own_both());
+    assert!(!ks.is_empty());
+    assert!(!ks.is_never_mind());
+    assert!(!ks.is_nothing_to_see_here());
+
+    let ks: KitchenSink<u16, u8> = KitchenSink::Empty;
+    assert!(!ks.is_left());
+    assert!(!ks.is_right());
+    assert!(!ks.is_own_both());
+    assert!(ks.is_empty());
+    assert!(!ks.is_never_mind());
+    assert!(!ks.is_nothing_to_see_here());
+
+    let ks: KitchenSink<u16, u8> = KitchenSink::NeverMind();
+    assert!(!ks.is_left());
+    assert!(!ks.is_right());
+    assert!(!ks.is_own_both());
+    assert!(!ks.is_empty());
+    assert!(ks.is_never_mind());
+    assert!(!ks.is_nothing_to_see_here());
+
+    let ks: KitchenSink<u16, u8> = KitchenSink::NothingToSeeHere {};
+    assert!(!ks.is_left());
+    assert!(!ks.is_right());
+    assert!(!ks.is_own_both());
+    assert!(!ks.is_empty());
+    assert!(!ks.is_never_mind());
+    assert!(ks.is_nothing_to_see_here());
+};