rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.98k stars 12.68k forks source link

Type and const parameters are not used as a fallback during type inference #98931

Open WalterSmuts opened 2 years ago

WalterSmuts commented 2 years ago

Currently you're able to define default const parameters for a type. This is explained here. This allows one to write generic impl blocks but requires some explicit type coercion as shown in this stack-overflow article on the new constructor.

It would be great if rust can infer that the type has to be the return type of the new constructor and, if the const parameter is not provided, infer that the type defaults to the default const value.

The stack-overflow article had me confused for a bit (see this reddit post) so I'll provide a more explicit example:

#[derive(Debug)]
struct Foo<const N: usize = 1> {
    set_on_creation: bool,
    passed_in: bool,
    generic_field: [usize; N],
}

impl<const N: usize> Foo<N> {
    pub fn new(passed_in: bool) -> Self {
        Self {
            set_on_creation: true,
            passed_in,
            generic_field: [0; N],
        }
    }
}

fn main() {
    let a = Foo::new(true);
    dbg!(a);
    let b = Foo::new(false);
    dbg!(b);
    let c: Foo<2> = Foo::new(false);
    dbg!(c);
}

I expected to see this happen: Successfully infer that the type has to be the default const variant of the type.

Instead, this happened:

error[E0282]: type annotations needed for `Foo<N>`
  --> src/main.rs:19:13
   |
19 |     let a = Foo::new(true);
   |         -   ^^^^^^^^ cannot infer the value of const parameter `N`
   |         |
   |         consider giving `a` the explicit type `Foo<N>`, where the const parameter `N` is specified

Clarity

To be clear, the following works:

    let a: Foo = Foo::new(true);

but this does not:

    let a = Foo::new(true);

On the face of it the above looks silly.

Meta

Bug is also present on nightly.

[walter@cuddles const-generic-default-playground]$ rustc --version --verbose
rustc 1.60.0 (7737e0b5c 2022-04-04)
binary: rustc
commit-hash: 7737e0b5c4103216d6fd8cf941b7ab9bdbaace7c
commit-date: 2022-04-04
host: x86_64-unknown-linux-gnu
release: 1.60.0
LLVM version: 14.0.0
fmease commented 2 years ago

Duplicate of #83687, #96300.

JulianKnodt commented 2 years ago

@fmease can we move these to one issue, and I can try to fix it? I didn't realize this didn't work

fmease commented 2 years ago

@JulianKnodt The thing is – if I remember correctly – that this issue / feature isn't a trivial one to fix / implement and that it first requires some design work / large architectural changes to rustc's inference engine. This topic has already been discussed somewhere sometime ago (sadly I can't link to anything). There might also be concerns around backward compatibility and inference ambiguities.

I would suggest you to search and ask on Zulip or on the internals forum for some context, background information or initial guidance maybe.

fmease commented 2 years ago

It would probably also require a major change proposal (MCP) or an RFC.

JulianKnodt commented 2 years ago

Chalk currently has no way to handle generic consts, and I was working on adding that (altho I got sidetracked and haven't actually put in work to get it closer to working), but it would still be nice to track these issues under const-generics or const_generics_defaults

WalterSmuts commented 2 years ago

@JulianKnodt Please can you fix this! IMO this is such a sore finger in rust language.

FYI, this probably needs to go hand-in hand with default generic type inference too. For example see the following code on normal generic (non-const generic) types:

#[derive(Default)]
struct MyTypeWrapper<T = i32>(T);

impl<T> MyTypeWrapper<T> {
    const fn new(t: T) -> Self {
        Self(t)
    }
}

fn main() {
    // This does work but is needlessly verbose IMO
    let a: MyTypeWrapper = MyTypeWrapper::default();

    // This does not work but should IMO
    let a = MyTypeWrapper::default();

    // This should work and does work
    let a: MyTypeWrapper<usize> = MyTypeWrapper::default();
}

The fact that rust behaves like this for normal generic default types is why https://github.com/rust-lang/rust/issues/83687 was closed.

JulianKnodt commented 2 years ago

Hmmmm, I didn't realize that generic default types also behaved that way, but it might be worth it to just add a note to the error message.

Edit: I'm wondering whether it would be possible to just fix this by first trying to infer the generic args without defaults, and if it fails substitute defaults one by one and see if any succeeds?

TimJentzsch commented 2 years ago

We ran into an issue that would benefit from const generic defaults working on generic functions as well.

We had a struct with a field of petitset::PetitSet which itself has a const generic maximum size. In our code, this was hard-coded to 8. We now wanted to make this generic instead, to give the user more control of the maximum size. To avoid ergonomic regressions and breaking changes, we planned to provide a default value of 8 to the generic, so that it would be a seamless transitions for the users.

However, because the default generic value cannot be derived for the constructor methods, the users would have to add a new type annotation or specify the size explicitly to avoid errors. Thus, it would be a breaking change and be more verbose to write, so we have to hold back this feature for now.

Related PR is at https://github.com/Leafwing-Studios/leafwing-input-manager/pull/241

JulianKnodt commented 2 years ago

@TimJentzsch After some discussion about it, it would make sense to add a warning that if the parameter is missing but has a default, it still needs to be explicitly specified. Instead, what you may be able to do is have two constructors, one specifically implemented for your default:

impl YourStruct<8 /* Or whatever your default is */> {
  fn new() -> Self {
    ...
  }
}

And one generic one:

impl<const N: usize> YourStruct<N> {
  fn new_non_default() -> Self {
     ...
  }
}

While it is less elegant to have 2 separate constructors with different names, it should be back-compatible and will allow you to differentiate and have changes for different sizes between the new constructor and the old constructor without breaking older users.

Playground Reference

alice-i-cecile commented 2 years ago

That's an okay (if inelegant) workaround for now. Is it impossible / impractical to make this default value inferred?

JulianKnodt commented 2 years ago

I think currently there is no movement to do so, because it would lead to a mismatch between inferred consts and inferred type, neither of which is inferred on an impl. I also personally feel that in this case inference to the default makes sense and is intuitive, but I can't make the change on my own, it would require an MCP as @fmease says

MoSal commented 1 year ago

This limitation can be weird sometimes.

struct Foo<const A: usize, const B: usize = 0>;

impl<const B: usize> Foo<0, B> {
    fn mk() -> Self { Self }
}

fn main() {
    // complains about unspecified B
    let f = Foo::mk();

    // works, and that zero is not B's
    let f: Foo<0> = Foo::mk();
}
fmease commented 9 months ago

I'm now using this as a hub for duplicate bug reports and for discussions around #![feature(default_type_parameter_fallback)] (https://github.com/rust-lang/rust/labels/F-default_type_parameter_fallback) superseding the closed tracking issue #27336.

goffrie commented 3 weeks ago

Oddly, it does work if you use the syntax <Foo>::new(), or define a type alias type Bar = Foo; Bar::new().

fmease commented 3 weeks ago

Right, that's because the omission of path arguments is handled differently depending on the context, namely on whether they're in "type position" or in "expression position".

In type position, no type inference occurs when "expanding" the generic arguments and one has to supply arguments for all non-lifetime non-defaulted parameters and parameter defaults are considered (for lifetime parameters, elision is permitted (well, in even fewer contexts that is)). That's your <Ty>::assoc and your type Al = Ty.

In expression position, a lack of generic arguments leads to the compiler supplying inference variables for all generic parameters (I'm ignoring lifetime elision here). These inference variables don't know a lot about their origin starting out, most notably the fact that there could be "defaults" for them. They are disconnected from that notion, so naturally there's no way for the type checker to consider the defaults.

And while we could almost trivially equip their origin (e.g. TypeVariableOrigin in the compiler) with a notion of defaults, the problem lies in the question of when exactly to apply said default during the inference process. This is why there has been no progress whatsoever on this feature.