Option to make constructor const

[ids1024]

It's sometimes convenient for a constructor to be a const fn.

Compiling the unit test here fails with:

   Compiling derive-new v0.5.9 (/home/ian/src/derive-new)
error[E0659]: `new` is ambiguous (derive helper attribute vs any other name)
  --> tests/test.rs:36:3
   |
36 | #[new(const)]
   |   ^^^ ambiguous name
   |
note: `new` could refer to the derive helper attribute defined here
  --> tests/test.rs:35:10
   |
35 | #[derive(new)]
   |          ^^^
note: `new` could also refer to the derive macro imported here
  --> tests/test.rs:3:1
   |
3  | #[macro_use]
   | ^^^^^^^^^^^^

Not sure how to solve that. I guess not using the same name new for both...

I guess if that can be addressed, this should also have documentation, and work with enums. Anyway, I thought I might as well open a draft PR with the code I have for now.

[nrc]

Could the constructor always be const? I think that anything we generate should be const-able? Maybe with the more complicated options we would have to make them not const to be conservative?

[ids1024]

Good question. I was thinking of types like String that involve allocation and can't be constructed in a const context. But that doesn't seem to be a problem. const fn f(s: String) {} fails to compile because it would invoke a destructor, but const fn f(s: String) -> String { s } compiles fine, though it wouldn't be possible to actually call it in a const context.

Maybe there are other edge cases to beware of though.

[ids1024]

Oh, bigger issue: Default::default is (unsurprisingly) not a const fn. So presumably a lot of uses of this crate break it it were always const. That happened to not be a problem for the use case I was testing.

[nrc]

Oh, bigger issue: Default::default is (unsurprisingly) not a const fn. So presumably a lot of uses of this crate break it it were always const.

Why is this an issue?

[ids1024]

Using #[new(const)] from this branch when there's a #[new(default)] field fails to compile with:

error[E0723]: can only call other `const fn` within a `const fn`, but `<i32 as std::default::Default>::default` is not stable as `const fn`

[nrc]

I see, thanks! How about we make new always const unless default is specified?

[ids1024]

Looking at https://github.com/nrc/derive-new#examples, #[new(value = "vec![1]")] would presumably also fail to compile as a const fn, and while #[new(value = "42")] would be fine. Which is not really possible for the macro to detect.

So I think it's probably necessary to make it an option of some kind.

[nrc]

I think that if there are no field annotations, then we can always generate a const fn? Perhaps we could so something like allow #[new(const value = "...")] which means that the user promises that the expression is a const expression, then we generate const if: no fields are default and no fields are non-const values.

(The reason I'm digging into this is that in general I think const should be the default, not opt-in because otherwise there will be a lot of consts on otherwise simple data, and there is no downside to a function being const where possible. Having annotations on the struct itself is difficult, maybe even impossible, whereas having annotations on fields is easy).

[ids1024]

I think that should work.

It is potentially a backwards compatability issue though. (The Readme specifically mentions adding a new member without breaking comparability as a goal of the crate.) Adding a use of #[new(const)] would break dependent code that uses it in a const context. And a member with a type like String couldn't be given a default value with #[new(const value = "...")] either.

Having annotations on the struct itself is difficult, maybe even impossible

What do you mean? It's difficult to the extent that the code is somewhat verbose, but it's certainly not impossible. The code in this PR works, for structs, except for the conflict when the new macro is in scope. Attributes on a struct with derive macros don't seem to be uncommon (for instance in serde). Normally a name conflict like this doesn't happen because the derive macro has an snake case name while the attribute is lower case.

I suppose this issue would be addressed using another attribute name, like #[new_const].

[nrc]

It is potentially a backwards compatability issue though. (The Readme specifically mentions adding a new member without breaking comparability as a goal of the crate.) Adding a use of #[new(const)] would break dependent code that uses it in a const context.

AIUI, adding a new member would be backwards compatible as would adding #[new(const value ...]. #[new(const)] would just be a syntax error.

And a member with a type like String couldn't be given a default value with #[new(const value = "...")] either.

If there is a field with type String, then with no annotations, we would generate a const ctor. Using #[new(const value = "...")] would give a compile time error, #[new(value = "...")] would force a non-const ctor; in theory that breaks users calling the ctor from const context, but adding that annotation means that the signature of the ctor changes, so their code breaks in any case.

What do you mean?

The conflict with new is what I meant here - I think it will nearly always be in scope. This is the edge of my macro/derive knowledge though, so I might well be wrong that there is an issue here.

[brunoschmidt]

Sorry to bring this one back from the dead, but have you considered that changing the field attribute is not the right move, since the "const" will need to be repeated every annotated field?

Wouldn't it be better to create a new derive, which could be "new_const", that uses the same atributes but creates a const new.
This solves the compatibility and repetition problems.

This also fixes #56 and #29.