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

Lower the priority of unstable methods when picking a candidate. #48552

Merged
merged 6 commits into from
Mar 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ declare_lint! {
"floating-point literals cannot be used in patterns"
}

declare_lint! {
pub UNSTABLE_NAME_COLLISION,
Warn,
"detects name collision with an existing but unstable method"
}

/// 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 @@ -307,7 +313,8 @@ impl LintPass for HardwiredLints {
SINGLE_USE_LIFETIME,
TYVAR_BEHIND_RAW_POINTER,
ELIDED_LIFETIME_IN_PATH,
BARE_TRAIT_OBJECT
BARE_TRAIT_OBJECT,
UNSTABLE_NAME_COLLISION,
)
}
}
Expand Down
20 changes: 13 additions & 7 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,15 +498,21 @@ pub fn struct_lint_level<'a>(sess: &'a Session,

// Check for future incompatibility lints and issue a stronger warning.
let lints = sess.lint_store.borrow();
if let Some(future_incompatible) = lints.future_incompatible(LintId::of(lint)) {
let future = if let Some(edition) = future_incompatible.edition {
format!("the {} edition", edition)
let lint_id = LintId::of(lint);
if let Some(future_incompatible) = lints.future_incompatible(lint_id) {
const STANDARD_MESSAGE: &str =
"this was previously accepted by the compiler but is being phased out; \
it will become a hard error";

let explanation = if lint_id == LintId::of(::lint::builtin::UNSTABLE_NAME_COLLISION) {
"once this method is added to the standard library, \
there will be ambiguity here, which will cause a hard error!"
.to_owned()
} else if let Some(edition) = future_incompatible.edition {
format!("{} in the {} edition!", STANDARD_MESSAGE, edition)
} else {
"a future release".to_owned()
format!("{} in a future release!", STANDARD_MESSAGE)
};
let explanation = format!("this was previously accepted by the compiler \
but is being phased out; \
it will become a hard error in {}!", future);
let citation = format!("for more information, see {}",
future_incompatible.reference);
err.warn(&explanation);
Expand Down
118 changes: 80 additions & 38 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,22 @@ struct Checker<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
}

/// Result of `TyCtxt::eval_stability`.
pub enum EvalResult {
/// We can use the item because it is stable or we provided the
/// corresponding feature gate.
Allow,
/// We cannot use the item because it is unstable and we did not provide the
/// corresponding feature gate.
Deny {
feature: Symbol,
reason: Option<Symbol>,
issue: u32,
},
/// The item does not have the `#[stable]` or `#[unstable]` marker assigned.
Unmarked,
}

impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// (See issue #38412)
fn skip_stability_check_due_to_privacy(self, mut def_id: DefId) -> bool {
Expand Down Expand Up @@ -509,14 +525,23 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) {
/// Evaluates the stability of an item.
///
/// Returns `EvalResult::Allow` if the item is stable, or unstable but the corresponding
/// `#![feature]` has been provided. Returns `EvalResult::Deny` which describes the offending
/// unstable feature otherwise.
///
/// If `id` is `Some(_)`, this function will also check if the item at `def_id` has been
/// deprecated. If the item is indeed deprecated, we will emit a deprecation lint attached to
/// `id`.
pub fn eval_stability(self, def_id: DefId, id: Option<NodeId>, span: Span) -> EvalResult {
if span.allows_unstable() {
debug!("stability: \
skipping span={:?} since it is internal", span);
return;
return EvalResult::Allow;
}

let lint_deprecated = |def_id: DefId, note: Option<Symbol>| {
let lint_deprecated = |def_id: DefId, id: NodeId, note: Option<Symbol>| {
let path = self.item_path_str(def_id);

let msg = if let Some(note) = note {
Expand All @@ -526,30 +551,29 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
};

self.lint_node(lint::builtin::DEPRECATED, id, span, &msg);
if id == ast::DUMMY_NODE_ID {
span_bug!(span, "emitted a deprecated lint with dummy node id: {:?}", def_id);
}
};

// Deprecated attributes apply in-crate and cross-crate.
if let Some(depr_entry) = self.lookup_deprecation_entry(def_id) {
let skip = if id == ast::DUMMY_NODE_ID {
true
} else {
if let Some(id) = id {
if let Some(depr_entry) = self.lookup_deprecation_entry(def_id) {
let parent_def_id = self.hir.local_def_id(self.hir.get_parent(id));
self.lookup_deprecation_entry(parent_def_id).map_or(false, |parent_depr| {
parent_depr.same_origin(&depr_entry)
})
let skip = self.lookup_deprecation_entry(parent_def_id)
.map_or(false, |parent_depr| parent_depr.same_origin(&depr_entry));
if !skip {
lint_deprecated(def_id, id, depr_entry.attr.note);
}
};

if !skip {
lint_deprecated(def_id, depr_entry.attr.note);
}
}

let is_staged_api = self.lookup_stability(DefId {
index: CRATE_DEF_INDEX,
..def_id
}).is_some();
if !is_staged_api {
return;
return EvalResult::Allow;
}

let stability = self.lookup_stability(def_id);
Expand All @@ -558,26 +582,26 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {

if let Some(&Stability{rustc_depr: Some(attr::RustcDeprecation { reason, .. }), ..})
= stability {
if id != ast::DUMMY_NODE_ID {
lint_deprecated(def_id, Some(reason));
if let Some(id) = id {
lint_deprecated(def_id, id, Some(reason));
}
}

// Only the cross-crate scenario matters when checking unstable APIs
let cross_crate = !def_id.is_local();
if !cross_crate {
return
return EvalResult::Allow;
}

// Issue 38412: private items lack stability markers.
if self.skip_stability_check_due_to_privacy(def_id) {
return
return EvalResult::Allow;
}

match stability {
Some(&Stability { level: attr::Unstable {ref reason, issue}, ref feature, .. }) => {
if self.stability().active_features.contains(feature) {
return
Some(&Stability { level: attr::Unstable { reason, issue }, feature, .. }) => {
if self.stability().active_features.contains(&feature) {
return EvalResult::Allow;
}

// When we're compiling the compiler itself we may pull in
Expand All @@ -589,19 +613,41 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// the `-Z force-unstable-if-unmarked` flag present (we're
// compiling a compiler crate), then let this missing feature
// annotation slide.
if *feature == "rustc_private" && issue == 27812 {
if feature == "rustc_private" && issue == 27812 {
if self.sess.opts.debugging_opts.force_unstable_if_unmarked {
return
return EvalResult::Allow;
}
}

let msg = match *reason {
Some(ref r) => format!("use of unstable library feature '{}': {}",
feature.as_str(), &r),
EvalResult::Deny { feature, reason, issue }
}
Some(_) => {
// Stable APIs are always ok to call and deprecated APIs are
// handled by the lint emitting logic above.
EvalResult::Allow
}
None => {
EvalResult::Unmarked
}
}
}

/// Checks if an item is stable or error out.
///
/// If the item defined by `def_id` is unstable and the corresponding `#![feature]` does not
/// exist, emits an error.
///
/// Additionally, this function will also check if the item is deprecated. If so, and `id` is
/// not `None`, a deprecated lint attached to `id` will be emitted.
pub fn check_stability(self, def_id: DefId, id: Option<NodeId>, span: Span) {
match self.eval_stability(def_id, id, span) {
EvalResult::Allow => {}
EvalResult::Deny { feature, reason, issue } => {
let msg = match reason {
Some(r) => format!("use of unstable library feature '{}': {}", feature, r),
None => format!("use of unstable library feature '{}'", &feature)
};


let msp: MultiSpan = span.into();
let cm = &self.sess.parse_sess.codemap();
let span_key = msp.primary_span().and_then(|sp: Span|
Expand All @@ -624,12 +670,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
GateIssue::Library(Some(issue)), &msg);
}
}
Some(_) => {
// Stable APIs are always ok to call and deprecated APIs are
// handled by the lint emitting logic above.
}
None => {
span_bug!(span, "encountered unmarked API");
EvalResult::Unmarked => {
span_bug!(span, "encountered unmarked API: {:?}", def_id);
}
}
}
Expand All @@ -655,7 +697,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
None => return,
};
let def_id = DefId { krate: cnum, index: CRATE_DEF_INDEX };
self.tcx.check_stability(def_id, item.id, item.span);
self.tcx.check_stability(def_id, Some(item.id), item.span);
}

// For implementations of traits, check the stability of each item
Expand All @@ -668,8 +710,8 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
let trait_item_def_id = self.tcx.associated_items(trait_did)
.find(|item| item.name == impl_item.name).map(|item| item.def_id);
if let Some(def_id) = trait_item_def_id {
// Pass `DUMMY_NODE_ID` to skip deprecation warnings.
self.tcx.check_stability(def_id, ast::DUMMY_NODE_ID, impl_item.span);
// Pass `None` to skip deprecation warnings.
self.tcx.check_stability(def_id, None, impl_item.span);
}
}
}
Expand Down Expand Up @@ -705,7 +747,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
match path.def {
Def::Local(..) | Def::Upvar(..) |
Def::PrimTy(..) | Def::SelfTy(..) | Def::Err => {}
_ => self.tcx.check_stability(path.def.def_id(), id, path.span)
_ => self.tcx.check_stability(path.def.def_id(), Some(id), path.span)
}
intravisit::walk_path(self, path)
}
Expand Down
9 changes: 8 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,14 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
id: LintId::of(TYVAR_BEHIND_RAW_POINTER),
reference: "issue #46906 <https://github.com/rust-lang/rust/issues/46906>",
edition: Some(Edition::Edition2018),
}
},
FutureIncompatibleInfo {
id: LintId::of(UNSTABLE_NAME_COLLISION),
reference: "issue #48919 <https://github.com/rust-lang/rust/issues/48919>",
edition: None,
// Note: this item represents future incompatibility of all unstable functions in the
// standard library, and thus should never be removed or changed to an error.
},
]);

// Register renamed and removed lints
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
let msg = format!("associated type `{}` is private", binding.item_name);
tcx.sess.span_err(binding.span, &msg);
}
tcx.check_stability(assoc_ty.def_id, ref_id, binding.span);
tcx.check_stability(assoc_ty.def_id, Some(ref_id), binding.span);

Ok(candidate.map_bound(|trait_ref| {
ty::ProjectionPredicate {
Expand Down Expand Up @@ -868,7 +868,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
let msg = format!("{} `{}` is private", def.kind_name(), assoc_name);
tcx.sess.span_err(span, &msg);
}
tcx.check_stability(item.def_id, ref_id, span);
tcx.check_stability(item.def_id, Some(ref_id), span);

(ty, def)
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -861,7 +861,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects");
let field_ty = self.field_ty(subpat.span, &variant.fields[i], substs);
self.check_pat_walk(&subpat, field_ty, def_bm, true);

self.tcx.check_stability(variant.fields[i].did, pat.id, subpat.span);
self.tcx.check_stability(variant.fields[i].did, Some(pat.id), subpat.span);
}
} else {
let subpats_ending = if subpats.len() == 1 { "" } else { "s" };
Expand Down Expand Up @@ -923,7 +923,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects");
vacant.insert(span);
field_map.get(&field.name)
.map(|f| {
self.tcx.check_stability(f.did, pat_id, span);
self.tcx.check_stability(f.did, Some(pat_id), span);

self.field_ty(span, f, substs)
})
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::traits::ObligationCause;

use syntax::ast;
use syntax::util::parser::PREC_POSTFIX;
use syntax_pos::{self, Span};
use syntax_pos::Span;
use rustc::hir;
use rustc::hir::def::Def;
use rustc::hir::map::NodeItem;
Expand Down Expand Up @@ -140,7 +140,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
if let Some((msg, suggestion)) = self.check_ref(expr, checked_ty, expected) {
err.span_suggestion(expr.span, msg, suggestion);
} else if !self.check_for_cast(&mut err, expr, expr_ty, expected) {
let methods = self.get_conversion_methods(expected, checked_ty);
let methods = self.get_conversion_methods(expr.span, expected, checked_ty);
if let Ok(expr_text) = self.tcx.sess.codemap().span_to_snippet(expr.span) {
let suggestions = iter::repeat(expr_text).zip(methods.iter())
.map(|(receiver, method)| format!("{}.{}()", receiver, method.name))
Expand All @@ -155,9 +155,9 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
(expected, Some(err))
}

fn get_conversion_methods(&self, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
fn get_conversion_methods(&self, span: Span, expected: Ty<'tcx>, checked_ty: Ty<'tcx>)
-> Vec<AssociatedItem> {
let mut methods = self.probe_for_return_type(syntax_pos::DUMMY_SP,
let mut methods = self.probe_for_return_type(span,
probe::Mode::MethodCall,
expected,
checked_ty,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
.unwrap().insert(import_def_id);
}

self.tcx.check_stability(pick.item.def_id, call_expr.id, span);
self.tcx.check_stability(pick.item.def_id, Some(call_expr.id), span);

let result = self.confirm_method(span,
self_expr,
Expand Down Expand Up @@ -371,7 +371,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
}

let def = pick.item.def();
self.tcx.check_stability(def.def_id(), expr_id, span);
self.tcx.check_stability(def.def_id(), Some(expr_id), span);

Ok(def)
}
Expand Down
Loading