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

Generic type defaults should not be allowed if they don’t satisfy type constraints #28024

Closed
nagisa opened this issue Aug 26, 2015 · 9 comments
Assignees
Labels
A-type-system Area: Type system C-bug Category: This is a bug. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented Aug 26, 2015

As spotted in this internals thread:

use std::fmt::Debug;

struct Foo;

fn show<T: Debug = Foo>(t: T) {
    println!("{:?}", t)
}

compiles. Lack of early error/warning/lint here might be hiding bugs in API design if the default never actually kicks in inside the library.

@alexcrichton
Copy link
Member

triage: I-nominated

@alexcrichton alexcrichton added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 9, 2015
@nikomatsakis nikomatsakis self-assigned this Oct 1, 2015
@nikomatsakis
Copy link
Contributor

triage: P-high

Seems like a straightforward oversight in the WF checker.

@rust-highfive rust-highfive added P-high High priority and removed I-nominated labels Oct 1, 2015
@nikomatsakis
Copy link
Contributor

Hmm, thinking more about this, it's perhaps not as straightforward as I originally thought. It gets into problems much like higher-ranked regions etc.

For example:

fn foo<T,U = T>()
    where U: Clone
{
}

Here, we can't fully check the value of the default ahead of time, because it references another type parameter (T), and we don't know yet whether T: Clone holds or not. I think for maximal consistency with rust-lang/rfcs#1214, though, we would strive to check where possible, and not otherwise.

cc @rust-lang/lang

@nikomatsakis
Copy link
Contributor

Nominating for discussion.

@nikomatsakis
Copy link
Contributor

I'm also going to lower the triage. It seems unlikely that anyone would intentionally abuse this, and I think we'll be able to fix this without undue anger or breakage out there in the wild.

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated P-high High priority labels Oct 7, 2015
@brson brson added the A-type-system Area: Type system label Jul 14, 2016
@brson
Copy link
Contributor

brson commented Jul 14, 2016

Triage: still a problem.

@nikomatsakis Long time since any update. Do you know how to fix this and can mentor?

@steveklabnik
Copy link
Member

steveklabnik commented Jul 14, 2016

So it's worth noting that this will compile (modulo some minor fixes), but, if you try to actually use show(), it won't work:

struct Foo;

fn show<T: std::fmt::Debug = Foo>(t: T) {
    println!("{:?}", t)
}

fn main() {
    let f = Foo;
    show(f);
}

gives

error: the trait bound `Foo: std::fmt::Debug` is not satisfied [--explain E0277]
 --> <anon>:9:5
9 |>     show(f);
  |>     ^^^^
note: `Foo` cannot be formatted using `:?`; if it is defined in your crate, add `#[derive(Debug)]` or manually implement it
note: required by `show`

@nikomatsakis
Copy link
Contributor

In general, type defaults have been kind of stalled; we've been wanting to rethink the design overall, which has led to a lack of motivation to fixing details of the implementation. I wouldn't in any case tackle this bug first -- the overall algorithm isn't really following the RFC, so I'd fix that first. Not sure if there's an issue about that though.

@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 22, 2017
@bstrie
Copy link
Contributor

bstrie commented Feb 20, 2020

Triage: the given example properly fails to compile, giving:

error: defaults for type parameters are only allowed in `struct`, `enum`, `type`, or `trait` definitions.
 --> src/lib.rs:5:9
  |
5 | fn show<T: Debug = Foo>(t: T) {
  |         ^
  |
  = note: `#[deny(invalid_type_param_default)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #36887 <https://github.com/rust-lang/rust/issues/36887>

...however, it fails to compile not because the compiler is detecting the invalid default, but because type defaults on functions are being phased out (#36887). Therefore we must ask whether or not it's possible to exhibit an unenforced type default using some other means.

My simple attempt to exhibit this bug without using functions has been properly prevented:

use std::fmt::Debug;

struct Foo;

trait Bar<T: Debug = Foo> {}
error[E0277]: `Foo` doesn't implement `std::fmt::Debug`
 --> src/lib.rs:5:14
  |
5 | trait Bar<T: Debug = Foo> {}
  | -------------^^^^^-------
  | |            |
  | |            `Foo` cannot be formatted using `{:?}`
  | required by `Bar`
  |
  = help: the trait `std::fmt::Debug` is not implemented for `Foo`
  = note: add `#[derive(Debug)]` or manually implement `std::fmt::Debug`

In light of there being no obvious way to reproduce the bug given our modern constraints, and since the bug is quite innocuous (AFAICT it can't ever lead to any actual runtime error, only to an unanticipated compiler error), I feel comfortable closing this.

@bstrie bstrie closed this as completed Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system C-bug Category: This is a bug. P-medium Medium priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants