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

Add functionality for epoch lints; add epoch lint for dyn-trait #48461

Merged
merged 8 commits into from
Mar 1, 2018
24 changes: 20 additions & 4 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use hir::HirVec;
use hir::map::{Definitions, DefKey, DefPathData};
use hir::def_id::{DefIndex, DefId, CRATE_DEF_INDEX, DefIndexAddressSpace};
use hir::def::{Def, PathResolution};
use lint::builtin::PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES;
use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES};
use middle::cstore::CrateStore;
use rustc_data_structures::indexed_vec::IndexVec;
use session::Session;
Expand Down Expand Up @@ -912,7 +912,11 @@ impl<'a> LoweringContext<'a> {
TyKind::Path(ref qself, ref path) => {
let id = self.lower_node_id(t.id);
let qpath = self.lower_qpath(t.id, qself, path, ParamMode::Explicit, itctx);
return self.ty_path(id, t.span, qpath);
let ty = self.ty_path(id, t.span, qpath);
if let hir::TyTraitObject(..) = ty.node {
self.maybe_lint_bare_trait(t.span, t.id, qself.is_none() && path.is_global());
}
return ty;
}
TyKind::ImplicitSelf => {
hir::TyPath(hir::QPath::Resolved(None, P(hir::Path {
Expand All @@ -931,7 +935,7 @@ impl<'a> LoweringContext<'a> {
let expr = self.lower_body(None, |this| this.lower_expr(expr));
hir::TyTypeof(expr)
}
TyKind::TraitObject(ref bounds, ..) => {
TyKind::TraitObject(ref bounds, kind) => {
let mut lifetime_bound = None;
let bounds = bounds.iter().filter_map(|bound| {
match *bound {
Expand All @@ -950,6 +954,9 @@ impl<'a> LoweringContext<'a> {
let lifetime_bound = lifetime_bound.unwrap_or_else(|| {
self.elided_lifetime(t.span)
});
if kind != TraitObjectSyntax::Dyn {
self.maybe_lint_bare_trait(t.span, t.id, false);
}
hir::TyTraitObject(bounds, lifetime_bound)
}
TyKind::ImplTrait(ref bounds) => {
Expand Down Expand Up @@ -3685,7 +3692,6 @@ impl<'a> LoweringContext<'a> {
// The original ID is taken by the `PolyTraitRef`,
// so the `Ty` itself needs a different one.
id = self.next_id();

hir::TyTraitObject(hir_vec![principal], self.elided_lifetime(span))
} else {
hir::TyPath(hir::QPath::Resolved(None, path))
Expand All @@ -3703,6 +3709,16 @@ impl<'a> LoweringContext<'a> {
name: hir::LifetimeName::Implicit,
}
}

fn maybe_lint_bare_trait(&self, span: Span, id: NodeId, is_global: bool) {
if self.sess.features.borrow().dyn_trait {
self.sess.buffer_lint_with_diagnostic(
builtin::BARE_TRAIT_OBJECT, id, span,
"trait objects without an explicit `dyn` are deprecated",
builtin::BuiltinLintDiagnostics::BareTraitObject(span, is_global)
)
}
}
}

fn body_ids(bodies: &BTreeMap<hir::BodyId, hir::Body>) -> Vec<hir::BodyId> {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
html_root_url = "https://doc.rust-lang.org/nightly/")]
#![deny(warnings)]

#![cfg_attr(not(stage0), allow(bare_trait_object))]

#![feature(box_patterns)]
#![feature(box_syntax)]
#![feature(conservative_impl_trait)]
Expand Down
39 changes: 37 additions & 2 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
//! compiler code, rather than using their own custom pass. Those
//! lints are all available in `rustc_lint::builtin`.

use errors::DiagnosticBuilder;
use lint::{LintPass, LateLintPass, LintArray};
use session::Session;
use session::config::Epoch;
use syntax::codemap::Span;

declare_lint! {
pub CONST_ERR,
Expand Down Expand Up @@ -252,6 +256,13 @@ declare_lint! {
"hidden lifetime parameters are deprecated, try `Foo<'_>`"
}

declare_lint! {
pub BARE_TRAIT_OBJECT,
Warn,
"suggest using `dyn Trait` for trait objects",
Epoch::Epoch2018
}

/// Does nothing as a lint pass, but registers some `Lint`s
/// which are used by other parts of the compiler.
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -298,10 +309,34 @@ impl LintPass for HardwiredLints {
COERCE_NEVER,
SINGLE_USE_LIFETIME,
TYVAR_BEHIND_RAW_POINTER,
ELIDED_LIFETIME_IN_PATH

ELIDED_LIFETIME_IN_PATH,
BARE_TRAIT_OBJECT
)
}
}

// this could be a closure, but then implementing derive traits
// becomes hacky (and it gets allocated)
#[derive(PartialEq, RustcEncodable, RustcDecodable, Debug)]
pub enum BuiltinLintDiagnostics {
Normal,
BareTraitObject(Span, /* is_global */ bool)
}

impl BuiltinLintDiagnostics {
pub fn run(self, sess: &Session, db: &mut DiagnosticBuilder) {
match self {
BuiltinLintDiagnostics::Normal => (),
BuiltinLintDiagnostics::BareTraitObject(span, is_global) => {
let sugg = match sess.codemap().span_to_snippet(span) {
Ok(ref s) if is_global => format!("dyn ({})", s),
Ok(s) => format!("dyn {}", s),
Err(_) => format!("dyn <type>")
};
db.span_suggestion(span, "use `dyn`", sugg);
}
}
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for HardwiredLints {}
48 changes: 39 additions & 9 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use self::TargetLint::*;
use std::slice;
use lint::{EarlyLintPassObject, LateLintPassObject};
use lint::{Level, Lint, LintId, LintPass, LintBuffer};
use lint::builtin::BuiltinLintDiagnostics;
use lint::levels::{LintLevelSets, LintLevelsBuilder};
use middle::privacy::AccessLevels;
use rustc_serialize::{Decoder, Decodable, Encoder, Encodable};
Expand Down Expand Up @@ -92,14 +93,19 @@ pub struct BufferedEarlyLint {
pub ast_id: ast::NodeId,
pub span: MultiSpan,
pub msg: String,
pub diagnostic: BuiltinLintDiagnostics,
}

/// Extra information for a future incompatibility lint. See the call
/// to `register_future_incompatible` in `librustc_lint/lib.rs` for
/// guidelines.
pub struct FutureIncompatibleInfo {
pub id: LintId,
pub reference: &'static str // e.g., a URL for an issue/PR/RFC or error code
/// e.g., a URL for an issue/PR/RFC or error code
pub reference: &'static str,
/// If this is an epoch fixing lint, the epoch in which
/// this lint becomes obsolete
pub epoch: Option<config::Epoch>,
}

/// The target of the `by_name` map, which accounts for renaming/deprecation.
Expand Down Expand Up @@ -194,11 +200,24 @@ impl LintStore {
pub fn register_future_incompatible(&mut self,
sess: Option<&Session>,
lints: Vec<FutureIncompatibleInfo>) {
let ids = lints.iter().map(|f| f.id).collect();
self.register_group(sess, false, "future_incompatible", ids);
for info in lints {
self.future_incompatible.insert(info.id, info);

for epoch in config::ALL_EPOCHS {
Copy link
Contributor

Choose a reason for hiding this comment

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

comment plz


"Create a lint group for each epoch corresponding to the future compatibility warnings targeting that epoch"


(Do you think that this ought to include the lints for "all previous epochs", since they are additive?)

(Also, are these lint groups insta-stable?)

Copy link
Member Author

Choose a reason for hiding this comment

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

We could. I think having them separate is cleaner, Rust doesn't do well with overlapping lint groups.

Rust lint stability is a very low bar, you just need to not remove the lint (aside from register_removed_lint() or whatever it's called). I think it's fine to insta-stable lint groups.

let lints = lints.iter().filter(|f| f.epoch == Some(*epoch)).map(|f| f.id)
.collect::<Vec<_>>();
if !lints.is_empty() {
self.register_group(sess, false, epoch.lint_name(), lints)
}
}

let mut future_incompatible = vec![];
for lint in lints {
future_incompatible.push(lint.id);
self.future_incompatible.insert(lint.id, lint);
}

self.register_group(sess, false, "future_incompatible", future_incompatible);


}

pub fn future_incompatible(&self, id: LintId) -> Option<&FutureIncompatibleInfo> {
Expand Down Expand Up @@ -429,6 +448,16 @@ pub trait LintContext<'tcx>: Sized {
self.lookup(lint, span, msg).emit();
}

fn lookup_and_emit_with_diagnostics<S: Into<MultiSpan>>(&self,
lint: &'static Lint,
span: Option<S>,
msg: &str,
diagnostic: BuiltinLintDiagnostics) {
let mut db = self.lookup(lint, span, msg);
diagnostic.run(self.sess(), &mut db);
db.emit();
}

fn lookup<S: Into<MultiSpan>>(&self,
lint: &'static Lint,
span: Option<S>,
Expand Down Expand Up @@ -499,9 +528,10 @@ impl<'a> EarlyContext<'a> {

fn check_id(&mut self, id: ast::NodeId) {
for early_lint in self.buffered.take(id) {
self.lookup_and_emit(early_lint.lint_id.lint,
Some(early_lint.span.clone()),
&early_lint.msg);
self.lookup_and_emit_with_diagnostics(early_lint.lint_id.lint,
Some(early_lint.span.clone()),
&early_lint.msg,
early_lint.diagnostic);
}
}
}
Expand Down Expand Up @@ -1054,7 +1084,7 @@ pub fn check_ast_crate(sess: &Session, krate: &ast::Crate) {
if !sess.opts.actually_rustdoc {
for (_id, lints) in cx.buffered.map {
for early_lint in lints {
span_bug!(early_lint.span, "failed to process buffered lint here");
sess.delay_span_bug(early_lint.span, "failed to process buffered lint here");
}
}
}
Expand Down
17 changes: 10 additions & 7 deletions src/librustc/lint/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,15 @@ impl LintLevelSets {
fn get_lint_level(&self,
lint: &'static Lint,
idx: u32,
aux: Option<&FxHashMap<LintId, (Level, LintSource)>>)
aux: Option<&FxHashMap<LintId, (Level, LintSource)>>,
sess: &Session)
-> (Level, LintSource)
{
let (level, mut src) = self.get_lint_id_level(LintId::of(lint), idx, aux);

// If `level` is none then we actually assume the default level for this
// lint.
let mut level = level.unwrap_or(lint.default_level);
let mut level = level.unwrap_or(lint.default_level(sess));

// If we're about to issue a warning, check at the last minute for any
// directives against the warnings "lint". If, for example, there's an
Expand Down Expand Up @@ -235,7 +236,8 @@ impl<'a> LintLevelsBuilder<'a> {
let lint = builtin::RENAMED_AND_REMOVED_LINTS;
let (level, src) = self.sets.get_lint_level(lint,
self.cur,
Some(&specs));
Some(&specs),
&sess);
lint::struct_lint_level(self.sess,
lint,
level,
Expand All @@ -248,7 +250,8 @@ impl<'a> LintLevelsBuilder<'a> {
let lint = builtin::UNKNOWN_LINTS;
let (level, src) = self.sets.get_lint_level(lint,
self.cur,
Some(&specs));
Some(&specs),
self.sess);
let msg = format!("unknown lint: `{}`", name);
let mut db = lint::struct_lint_level(self.sess,
lint,
Expand Down Expand Up @@ -342,7 +345,7 @@ impl<'a> LintLevelsBuilder<'a> {
msg: &str)
-> DiagnosticBuilder<'a>
{
let (level, src) = self.sets.get_lint_level(lint, self.cur, None);
let (level, src) = self.sets.get_lint_level(lint, self.cur, None, self.sess);
lint::struct_lint_level(self.sess, lint, level, src, span, msg)
}

Expand Down Expand Up @@ -377,11 +380,11 @@ impl LintLevelMap {
/// If the `id` was not previously registered, returns `None`. If `None` is
/// returned then the parent of `id` should be acquired and this function
/// should be called again.
pub fn level_and_source(&self, lint: &'static Lint, id: HirId)
pub fn level_and_source(&self, lint: &'static Lint, id: HirId, session: &Session)
-> Option<(Level, LintSource)>
{
self.id_to_set.get(&id).map(|idx| {
self.sets.get_lint_level(lint, *idx, None)
self.sets.get_lint_level(lint, *idx, None, session)
})
}

Expand Down
34 changes: 29 additions & 5 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ use errors::{DiagnosticBuilder, DiagnosticId};
use hir::def_id::{CrateNum, LOCAL_CRATE};
use hir::intravisit::{self, FnKind};
use hir;
use session::{Session, DiagnosticMessageId};
use lint::builtin::BuiltinLintDiagnostics;
use session::{config, Session, DiagnosticMessageId};
use std::hash;
use syntax::ast;
use syntax::codemap::MultiSpan;
Expand Down Expand Up @@ -74,25 +75,46 @@ pub struct Lint {
///
/// e.g. "imports that are never used"
pub desc: &'static str,

/// Deny lint after this epoch
pub epoch_deny: Option<config::Epoch>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Further down you state that the lint should be removed after the epoch, here it seems to suggest it should be deny.

Copy link
Member Author

Choose a reason for hiding this comment

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

Where is this? It used to say this, but that changed.

This specific lint becomes Deny in Rust2018. Other lints may wish to disappear entirely because they are no longer lints.

}

impl Lint {
/// Get the lint's name, with ASCII letters converted to lowercase.
pub fn name_lower(&self) -> String {
self.name.to_ascii_lowercase()
}

pub fn default_level(&self, session: &Session) -> Level {
if let Some(epoch_deny) = self.epoch_deny {
if session.epoch() >= epoch_deny {
return Level::Deny
}
}
self.default_level
}
}

/// Declare a static item of type `&'static Lint`.
#[macro_export]
macro_rules! declare_lint {
($vis: vis $NAME: ident, $Level: ident, $desc: expr, $epoch: expr) => (
$vis static $NAME: &$crate::lint::Lint = &$crate::lint::Lint {
name: stringify!($NAME),
default_level: $crate::lint::$Level,
desc: $desc,
epoch_deny: Some($epoch)
};
);
($vis: vis $NAME: ident, $Level: ident, $desc: expr) => (
$vis static $NAME: &$crate::lint::Lint = &$crate::lint::Lint {
name: stringify!($NAME),
default_level: $crate::lint::$Level,
desc: $desc
desc: $desc,
epoch_deny: None,
};
)
);
}

/// Declare a static `LintArray` and return it as an expression.
Expand Down Expand Up @@ -304,7 +326,7 @@ impl LintId {
/// Setting for how to handle a lint.
#[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)]
pub enum Level {
Allow, Warn, Deny, Forbid
Allow, Warn, Deny, Forbid,
}

impl_stable_hash_for!(enum self::Level {
Expand Down Expand Up @@ -378,12 +400,14 @@ impl LintBuffer {
lint: &'static Lint,
id: ast::NodeId,
sp: MultiSpan,
msg: &str) {
msg: &str,
diagnostic: BuiltinLintDiagnostics) {
let early_lint = BufferedEarlyLint {
lint_id: LintId::of(lint),
ast_id: id,
span: sp,
msg: msg.to_string(),
diagnostic
};
let arr = self.map.entry(id).or_insert(Vec::new());
if !arr.contains(&early_lint) {
Expand Down
Loading