Skip to content

Commit

Permalink
Auto merge of #30724 - nikomatsakis:feature-gate-defaulted-type-param…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
bors committed Jan 7, 2016
2 parents 4406717 + 6dd3f61 commit 91b27ec
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 19 deletions.
16 changes: 8 additions & 8 deletions src/libcollections/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1591,10 +1591,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[unstable(feature = "btree_range",
reason = "matches collection reform specification, waiting for dust to settle",
issue = "27787")]
pub fn range<Min: ?Sized + Ord = K, Max: ?Sized + Ord = K>(&self,
min: Bound<&Min>,
max: Bound<&Max>)
-> Range<K, V>
pub fn range<Min: ?Sized + Ord, Max: ?Sized + Ord>(&self,
min: Bound<&Min>,
max: Bound<&Max>)
-> Range<K, V>
where K: Borrow<Min> + Borrow<Max>
{
range_impl!(&self.root,
Expand Down Expand Up @@ -1633,10 +1633,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[unstable(feature = "btree_range",
reason = "matches collection reform specification, waiting for dust to settle",
issue = "27787")]
pub fn range_mut<Min: ?Sized + Ord = K, Max: ?Sized + Ord = K>(&mut self,
min: Bound<&Min>,
max: Bound<&Max>)
-> RangeMut<K, V>
pub fn range_mut<Min: ?Sized + Ord, Max: ?Sized + Ord>(&mut self,
min: Bound<&Min>,
max: Bound<&Max>)
-> RangeMut<K, V>
where K: Borrow<Min> + Borrow<Max>
{
range_impl!(&mut self.root,
Expand Down
8 changes: 4 additions & 4 deletions src/libcollections/btree/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ impl<T: Ord> BTreeSet<T> {
#[unstable(feature = "btree_range",
reason = "matches collection reform specification, waiting for dust to settle",
issue = "27787")]
pub fn range<'a, Min: ?Sized + Ord = T, Max: ?Sized + Ord = T>(&'a self,
min: Bound<&Min>,
max: Bound<&Max>)
-> Range<'a, T>
pub fn range<'a, Min: ?Sized + Ord, Max: ?Sized + Ord>(&'a self,
min: Bound<&Min>,
max: Bound<&Max>)
-> Range<'a, T>
where T: Borrow<Min> + Borrow<Max>
{
fn first<A, B>((a, _): (A, B)) -> A {
Expand Down
4 changes: 2 additions & 2 deletions src/libcore/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2132,7 +2132,7 @@ pub trait Iterator {
/// ```
#[unstable(feature = "iter_arith", reason = "bounds recently changed",
issue = "27739")]
fn sum<S=<Self as Iterator>::Item>(self) -> S where
fn sum<S>(self) -> S where
S: Add<Self::Item, Output=S> + Zero,
Self: Sized,
{
Expand All @@ -2157,7 +2157,7 @@ pub trait Iterator {
/// ```
#[unstable(feature="iter_arith", reason = "bounds recently changed",
issue = "27739")]
fn product<P=<Self as Iterator>::Item>(self) -> P where
fn product<P>(self) -> P where
P: Mul<Self::Item, Output=P> + One,
Self: Sized,
{
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

use lint::{LintPass, LateLintPass, LintArray};

// name of the future-incompatible group
pub const FUTURE_INCOMPATIBLE: &'static str = "future_incompatible";

declare_lint! {
pub CONST_ERR,
Warn,
Expand Down Expand Up @@ -124,6 +127,12 @@ declare_lint! {
"detect private items in public interfaces not caught by the old implementation"
}

declare_lint! {
pub INVALID_TYPE_PARAM_DEFAULT,
Warn,
"type parameter default erroneously allowed in invalid location"
}

/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
Expand All @@ -149,6 +158,7 @@ impl LintPass for HardwiredLints {
TRIVIAL_CASTS,
TRIVIAL_NUMERIC_CASTS,
PRIVATE_IN_PUBLIC,
INVALID_TYPE_PARAM_DEFAULT,
CONST_ERR
)
}
Expand Down
20 changes: 17 additions & 3 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,16 @@ pub fn gather_attrs(attrs: &[ast::Attribute])
/// lints elsewhere in the compiler should call
/// `Session::add_lint()` instead.
pub fn raw_emit_lint(sess: &Session,
lints: &LintStore,
lint: &'static Lint,
lvlsrc: LevelSource,
span: Option<Span>,
msg: &str) {
raw_struct_lint(sess, lint, lvlsrc, span, msg).emit();
raw_struct_lint(sess, lints, lint, lvlsrc, span, msg).emit();
}

pub fn raw_struct_lint<'a>(sess: &'a Session,
lints: &LintStore,
lint: &'static Lint,
lvlsrc: LevelSource,
span: Option<Span>,
Expand Down Expand Up @@ -414,6 +416,18 @@ pub fn raw_struct_lint<'a>(sess: &'a Session,
_ => sess.bug("impossible level in raw_emit_lint"),
};

// Check for future incompatibility lints and issue a stronger warning.
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!";
if let Some(sp) = span {
err.span_note(sp, msg);
} else {
err.note(msg);
}
}

if let Some(span) = def {
err.span_note(span, "lint level defined here");
}
Expand Down Expand Up @@ -451,7 +465,7 @@ pub trait LintContext: Sized {
Some(pair) => pair,
};

raw_emit_lint(&self.sess(), lint, (level, src), span, msg);
raw_emit_lint(&self.sess(), self.lints(), lint, (level, src), span, msg);
}

fn lookup(&self,
Expand All @@ -464,7 +478,7 @@ pub trait LintContext: Sized {
Some(pair) => pair,
};

raw_struct_lint(&self.sess(), lint, (level, src), span, msg)
raw_struct_lint(&self.sess(), self.lints(), lint, (level, src), span, msg)
}

/// Emit a lint at the appropriate level, for a particular span.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
UNUSED_MUT, UNREACHABLE_CODE, UNUSED_MUST_USE,
UNUSED_UNSAFE, PATH_STATEMENTS, UNUSED_ATTRIBUTES);

add_lint_group!(sess, "future_incompatible",
PRIVATE_IN_PUBLIC);
add_lint_group!(sess, FUTURE_INCOMPATIBLE,
PRIVATE_IN_PUBLIC, INVALID_TYPE_PARAM_DEFAULT);

// We have one lint pass defined specially
store.register_late_pass(sess, false, box lint::GatherNodeLevels);
Expand Down
1 change: 1 addition & 0 deletions src/librustc_trans/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2216,6 +2216,7 @@ fn enum_variant_size_lint(ccx: &CrateContext, enum_def: &hir::EnumDef, sp: Span,
// Use lint::raw_emit_lint rather than sess.add_lint because the lint-printing
// pass for the latter already ran.
lint::raw_struct_lint(&ccx.tcx().sess,
&ccx.tcx().sess.lint_store.borrow(),
lint::builtin::VARIANT_SIZE_DIFFERENCES,
*lvlsrc.unwrap(),
Some(sp),
Expand Down
12 changes: 12 additions & 0 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ There are some shortcomings in this design:
*/

use astconv::{self, AstConv, ty_of_arg, ast_ty_to_ty, ast_region_to_region};
use lint;
use middle::def;
use middle::def_id::DefId;
use constrained_type_params as ctp;
Expand Down Expand Up @@ -1910,6 +1911,17 @@ fn get_or_create_type_parameter_def<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>,

let parent = tcx.map.get_parent(param.id);

if space != TypeSpace && default.is_some() {
if !tcx.sess.features.borrow().default_type_parameter_fallback {
tcx.sess.add_lint(
lint::builtin::INVALID_TYPE_PARAM_DEFAULT,
param.id,
param.span,
format!("defaults for type parameters are only allowed \
on `struct` or `enum` definitions (see issue #27336)"));
}
}

let def = ty::TypeParameterDef {
space: space,
index: index,
Expand Down
1 change: 1 addition & 0 deletions src/test/auxiliary/default_ty_param_cross_crate_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#![crate_type = "lib"]
#![crate_name = "default_param_test"]
#![feature(default_type_parameter_fallback)]

use std::marker::PhantomData;

Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/issue-26812.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(default_type_parameter_fallback)]

fn avg<T=T::Item>(_: T) {} //~ ERROR associated type `Item` not found for `T`
fn main() {}
17 changes: 17 additions & 0 deletions src/test/compile-fail/type-parameter-invalid-lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![deny(future_incompatible)]
#![allow(dead_code)]

fn avg<T=i32>(_: T) {}
//~^ ERROR defaults for type parameters are only allowed
//~| NOTE HARD ERROR
fn main() {}

0 comments on commit 91b27ec

Please sign in to comment.