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

Feature gate defaulted type parameters appearing outside of types #30724

Merged

Conversation

nikomatsakis
Copy link
Contributor

It was recently realized that we accept defaulted type parameters everywhere, without feature gate, even though the only place that we really intended to accept them were on types. This PR adds a lint warning unless the "type-parameter-defaults" feature is enabled. This should eventually become a hard error.

This is a [breaking-change] in that new feature gates are required (or simply removing the defaults, which is probably a better choice as they have little effect at this time). Results of a crater run suggest that approximately 5-15 crates are affected. I didn't do the measurement quite right so that run cannot distinguish "true" regressions from "non-root" regressions, but even the upper bound of 15 affected crates seems relatively minimal.

cc @rust-lang/lang
r? @pnkfelix

@nikomatsakis nikomatsakis added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 5, 2016
let future_incompat_lints = &lints.lint_groups[builtin::FUTURE_INCOMPATIBLE];
let this_id = LintId::of(lint);
if future_incompat_lints.0.iter().any(|&id| id == this_id) {
let msg = "this lint will become a HARD ERROR in a future release!";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah cool, good to CSE this messaging in one place.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 5, 2016

@bors r+ 27d6b9d

@bors
Copy link
Contributor

bors commented Jan 5, 2016

🙀 27d6b9d is not a valid commit SHA. Please try again with 6dd3f61.

@pnkfelix
Copy link
Member

pnkfelix commented Jan 5, 2016

(graar i hate githubs presentation of the commit series)

@pnkfelix
Copy link
Member

pnkfelix commented Jan 5, 2016

@bors r+ 6dd3f61

@nikomatsakis
Copy link
Contributor Author

@bors r=pnkfelix

@bors
Copy link
Contributor

bors commented Jan 6, 2016

📌 Commit 6dd3f61 has been approved by pnkfelix

@bors
Copy link
Contributor

bors commented Jan 7, 2016

⌛ Testing commit 6dd3f61 with merge 91b27ec...

bors added a commit that referenced this pull request Jan 7, 2016
…eters, r=pnkfelix

It was recently realized that we accept defaulted type parameters everywhere, without feature gate, even though the only place that we really *intended* to accept them were on types. This PR adds a lint warning unless the "type-parameter-defaults" feature is enabled. This should eventually become a hard error.

This is a [breaking-change] in that new feature gates are required (or simply removing the defaults, which is probably a better choice as they have little effect at this time). Results of a [crater run][crater] suggest that approximately 5-15 crates are affected. I didn't do the measurement quite right so that run cannot distinguish "true" regressions from "non-root" regressions, but even the upper bound of 15 affected crates seems relatively minimal.

[crater]: https://gist.github.com/nikomatsakis/760c6a67698bd24253bf

cc @rust-lang/lang
r? @pnkfelix
@bors bors merged commit 6dd3f61 into rust-lang:master Jan 7, 2016
@pnkfelix pnkfelix added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 12, 2016
@pnkfelix
Copy link
Member

(I suspect we should not actually take any special effort to backport this to beta. But I'll leave the nominated tag to ensure that the team discusses it.)

@pnkfelix pnkfelix added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 12, 2016
@nikomatsakis nikomatsakis removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 12, 2016
@bluss
Copy link
Member

bluss commented Jan 13, 2016

@nikomatsakis I object to removing defaults in trait definitions. The description does not mention it either way, so I'm not 100% sure which way you are going here.

Something like the following is working fine for me today. It also allows backwards compatible addition of type parameters just like for structs.

trait IndexRange<T=usize> { fn start(&self) -> Option<T> }

fn index<R: IndexRange>(r: R) { ... }

@petrochenkov
Copy link
Contributor

@bluss
Defaults are still permitted on traits (as well as on impls, type aliases, enums and structs).
This PR restricts their use on functions and methods (and, technically, foreign functions).

@bluss
Copy link
Member

bluss commented Jan 13, 2016

@petrochenkov Ok, that's great. Some of the messages need updating for that.

@m4rw3r
Copy link
Contributor

m4rw3r commented Jan 13, 2016

Some functional programming with generics would benefit from having the default type parameters, provided that type-parameter-defaults is enabled. It would enable for eg. skipping error types or letting the user skip type annotations when functions would automatically convert types if no conversion is necessary (see Chomp's ret or bind for examples).

Since it is not yet enabled by default I will follow the recommendation and remove these defaults from Chomp.

@bluss
Copy link
Member

bluss commented Jan 13, 2016

type parameter defaults seem to be going rather than coming to rust, what it looks like now, @m4rw3r

m4rw3r added a commit to m4rw3r/chomp that referenced this pull request Jan 13, 2016
Manishearth added a commit to Manishearth/rust that referenced this pull request Jan 15, 2016
…nt, r=brson

There is now more structure to the report, so that you can specify e.g. an RFC/PR/issue number and other explanatory details.

Example message:

```
type-parameter-invalid-lint.rs:14:8: 14:9 error: defaults for type parameters are only allowed on type definitions, like `struct` or `enum`
type-parameter-invalid-lint.rs:14 fn avg<T=i32>(_: T) {}
                                         ^
type-parameter-invalid-lint.rs:14:8: 14:9 warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
type-parameter-invalid-lint.rs:14:8: 14:9 note: for more information, see PR 30742 <rust-lang#30724>
type-parameter-invalid-lint.rs:11:9: 11:28 note: lint level defined here
type-parameter-invalid-lint.rs:11 #![deny(future_incompatible)]
                                          ^~~~~~~~~~~~~~~~~~~
error: aborting due to previous error
```

r? @brson

I would really like feedback also on the specific messages!

Fixes rust-lang#30746
@bluss
Copy link
Member

bluss commented Jan 18, 2016

This should link to the tracking issue for default type parameter fallback: #27336 (now it does)

bluss added a commit to bluss/rust that referenced this pull request Jan 19, 2016
Introduced in PR rust-lang#30724, needs to mention that type parameter defaults
are legal in trait and type definitions too.
bors added a commit that referenced this pull request Jan 23, 2016
Fix type parameter default error to mention type and trait definitions

Introduced in PR #30724, needs to mention that type parameter defaults
are legal in trait and type definitions too.
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jan 23, 2016
Fix type parameter default error to mention type and trait definitions

Introduced in PR rust-lang#30724, needs to mention that type parameter defaults
are legal in trait and type definitions too.
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jan 23, 2016
Fix type parameter default error to mention type and trait definitions

Introduced in PR rust-lang#30724, needs to mention that type parameter defaults
are legal in trait and type definitions too.
bvssvni added a commit to bvssvni/graphics that referenced this pull request Feb 3, 2016
Closes PistonDevelopers#996

- Removed draw_state dependency
- Fixed [PR 30742](rust-lang/rust#30724)
(default generic parameters will become a hard error)
@nikomatsakis nikomatsakis deleted the feature-gate-defaulted-type-parameters branch March 30, 2016 16:13
trombonehero added a commit to musec/rusty-shark that referenced this pull request May 30, 2016
Allow the [un]signed functions to return integers smaller than [iu]64 and
also use the standard `byteorder` definitions of endianness rather than
implementing our own enum type.

Unfortunately, Rust functions are no longer allowed to have default type
arguments (see PR 30724, rust-lang/rust#30724),
so we still have to specify the endianness explicitly every time we use
one of these functions. The may require wrapping inside a type...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. 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.

6 participants