From abf4d8babfebcdad66599c50bf5b6ae3e5bcbeb5 Mon Sep 17 00:00:00 2001 From: kennytm Date: Mon, 26 Feb 2018 21:34:06 +0800 Subject: [PATCH] When picking a candidate, consider the unstable ones last. If there is potential ambiguity after stabilizing those candidates, a warning will be emitted. --- src/librustc/lint/builtin.rs | 9 +- src/librustc/middle/stability.rs | 72 +++++++---- src/librustc_lint/lib.rs | 10 +- src/librustc_typeck/check/method/probe.rs | 113 +++++++++++++++--- .../auxiliary/inference_unstable_iterator.rs | 24 ++++ .../auxiliary/inference_unstable_itertools.rs | 17 +++ src/test/ui/inference_unstable.rs | 29 +++++ src/test/ui/inference_unstable.stderr | 12 ++ src/test/ui/inference_unstable_featured.rs | 27 +++++ .../ui/inference_unstable_featured.stderr | 12 ++ src/test/ui/inference_unstable_forced.rs | 22 ++++ src/test/ui/inference_unstable_forced.stderr | 11 ++ 12 files changed, 317 insertions(+), 41 deletions(-) create mode 100644 src/test/ui/auxiliary/inference_unstable_iterator.rs create mode 100644 src/test/ui/auxiliary/inference_unstable_itertools.rs create mode 100644 src/test/ui/inference_unstable.rs create mode 100644 src/test/ui/inference_unstable.stderr create mode 100644 src/test/ui/inference_unstable_featured.rs create mode 100644 src/test/ui/inference_unstable_featured.stderr create mode 100644 src/test/ui/inference_unstable_forced.rs create mode 100644 src/test/ui/inference_unstable_forced.stderr diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index b4ed9c269bd..97cfcf0f607 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -260,6 +260,12 @@ "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)] @@ -307,7 +313,8 @@ fn get_lints(&self) -> LintArray { SINGLE_USE_LIFETIME, TYVAR_BEHIND_RAW_POINTER, ELIDED_LIFETIME_IN_PATH, - BARE_TRAIT_OBJECT + BARE_TRAIT_OBJECT, + UNSTABLE_NAME_COLLISION, ) } } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 16c33d6bd83..9193dc086fd 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -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, + 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 { @@ -509,11 +525,16 @@ fn skip_stability_check_due_to_privacy(self, mut def_id: DefId) -> bool { } } - pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { + /// Evaluates the stability of an item. + /// + /// Returns `None` if the item is stable, or unstable but the corresponding `#![feature]` has + /// been provided. Returns the tuple `Some((feature, reason, issue))` of the offending unstable + /// feature otherwise. + pub fn eval_stability(self, def_id: DefId, id: 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| { @@ -549,7 +570,7 @@ pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { ..def_id }).is_some(); if !is_staged_api { - return; + return EvalResult::Allow; } let stability = self.lookup_stability(def_id); @@ -566,18 +587,18 @@ pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { // 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 @@ -589,19 +610,34 @@ pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { // 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 + } + } + } + + pub fn check_stability(self, def_id: DefId, id: 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| @@ -624,12 +660,8 @@ pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) { 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); } } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 901d76edc80..fad21ecef4d 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -273,7 +273,15 @@ macro_rules! add_lint_group { id: LintId::of(TYVAR_BEHIND_RAW_POINTER), reference: "issue #46906 ", edition: Some(Edition::Edition2018), - } + }, + FutureIncompatibleInfo { + id: LintId::of(UNSTABLE_NAME_COLLISION), + reference: "pr #48552 ", + edition: None, + // FIXME: create a proper tracking issue. + // 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 diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index c7921d2bd45..738e1ee87c7 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -23,9 +23,10 @@ use rustc::infer::type_variable::TypeVariableOrigin; use rustc::util::nodemap::FxHashSet; use rustc::infer::{self, InferOk}; +use rustc::middle::stability; use syntax::ast; use syntax::util::lev_distance::{lev_distance, find_best_match_for_name}; -use syntax_pos::Span; +use syntax_pos::{Span, symbol::Symbol}; use rustc::hir; use rustc::lint; use std::mem; @@ -937,30 +938,59 @@ fn pick_method(&mut self, self_ty: Ty<'tcx>) -> Option> { debug!("pick_method(self_ty={})", self.ty_to_string(self_ty)); let mut possibly_unsatisfied_predicates = Vec::new(); - - debug!("searching inherent candidates"); - if let Some(pick) = self.consider_candidates(self_ty, - &self.inherent_candidates, - &mut possibly_unsatisfied_predicates) { - return Some(pick); + let mut unstable_candidates = Vec::new(); + + for (kind, candidates) in &[ + ("inherent", &self.inherent_candidates), + ("extension", &self.extension_candidates), + ] { + debug!("searching {} candidates", kind); + let res = self.consider_candidates( + self_ty, + candidates.iter(), + &mut possibly_unsatisfied_predicates, + Some(&mut unstable_candidates), + ); + if let Some(pick) = res { + if !unstable_candidates.is_empty() && !self_ty.is_ty_var() { + if let Ok(p) = &pick { + // Emit a lint if there are unstable candidates alongside the stable ones. + // + // Note, we suppress warning if `self_ty` is TyVar (`_`), since every + // possible candidates of every type will be considered, which leads to + // bogus ambiguity like `str::rsplit` vs `[_]::rsplit`. This condition is + // seen in `src/test/compile-fail/occurs-check-2.rs`. + self.emit_unstable_name_collision_hint(p, &unstable_candidates); + } + } + return Some(pick); + } } - debug!("searching extension candidates"); - let res = self.consider_candidates(self_ty, - &self.extension_candidates, - &mut possibly_unsatisfied_predicates); - if let None = res { + debug!("searching unstable candidates"); + let res = self.consider_candidates( + self_ty, + unstable_candidates.into_iter().map(|(c, _)| c), + &mut possibly_unsatisfied_predicates, + None, + ); + if res.is_none() { self.unsatisfied_predicates.extend(possibly_unsatisfied_predicates); } res } - fn consider_candidates(&self, - self_ty: Ty<'tcx>, - probes: &[Candidate<'tcx>], - possibly_unsatisfied_predicates: &mut Vec>) - -> Option> { - let mut applicable_candidates: Vec<_> = probes.iter() + fn consider_candidates<'b, ProbesIter>( + &self, + self_ty: Ty<'tcx>, + probes: ProbesIter, + possibly_unsatisfied_predicates: &mut Vec>, + unstable_candidates: Option<&mut Vec<(&'b Candidate<'tcx>, Symbol)>>, + ) -> Option> + where + ProbesIter: Iterator> + Clone, + { + let mut applicable_candidates: Vec<_> = probes.clone() .map(|probe| { (probe, self.consider_probe(self_ty, probe, possibly_unsatisfied_predicates)) }) @@ -975,8 +1005,20 @@ fn consider_candidates(&self, } } + if let Some(uc) = unstable_candidates { + applicable_candidates.retain(|&(p, _)| { + if let stability::EvalResult::Deny { feature, .. } = + self.tcx.eval_stability(p.item.def_id, ast::DUMMY_NODE_ID, self.span) + { + uc.push((p, feature)); + return false; + } + true + }); + } + if applicable_candidates.len() > 1 { - let sources = probes.iter() + let sources = probes .map(|p| self.candidate_source(p, self_ty)) .collect(); return Some(Err(MethodError::Ambiguity(sources))); @@ -991,6 +1033,39 @@ fn consider_candidates(&self, }) } + fn emit_unstable_name_collision_hint( + &self, + stable_pick: &Pick, + unstable_candidates: &[(&Candidate<'tcx>, Symbol)], + ) { + let mut diag = self.tcx.struct_span_lint_node( + lint::builtin::UNSTABLE_NAME_COLLISION, + self.fcx.body_id, + self.span, + "a method with this name will be added to the standard library in the future", + ); + + // FIXME: This should be a `span_suggestion` instead of `help`. However `self.span` only + // highlights the method name, so we can't use it. Also consider reusing the code from + // `report_method_error()`. + diag.help(&format!( + "call with fully qualified syntax `{}(...)` to keep using the current method", + self.tcx.item_path_str(stable_pick.item.def_id), + )); + + if ::rustc::session::config::nightly_options::is_nightly_build() { + for (candidate, feature) in unstable_candidates { + diag.note(&format!( + "add #![feature({})] to the crate attributes to enable `{}`", + feature, + self.tcx.item_path_str(candidate.item.def_id), + )); + } + } + + diag.emit(); + } + fn select_trait_candidate(&self, trait_ref: ty::TraitRef<'tcx>) -> traits::SelectionResult<'tcx, traits::Selection<'tcx>> { diff --git a/src/test/ui/auxiliary/inference_unstable_iterator.rs b/src/test/ui/auxiliary/inference_unstable_iterator.rs new file mode 100644 index 00000000000..b73346e6332 --- /dev/null +++ b/src/test/ui/auxiliary/inference_unstable_iterator.rs @@ -0,0 +1,24 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(staged_api)] + +#![stable(feature = "ipu_iterator", since = "1.0.0")] + +#[stable(feature = "ipu_iterator", since = "1.0.0")] +pub trait IpuIterator { + #[unstable(feature = "ipu_flatten", issue = "99999")] + fn ipu_flatten(&self) -> u32 { + 0 + } +} + +#[stable(feature = "ipu_iterator", since = "1.0.0")] +impl IpuIterator for char {} diff --git a/src/test/ui/auxiliary/inference_unstable_itertools.rs b/src/test/ui/auxiliary/inference_unstable_itertools.rs new file mode 100644 index 00000000000..2ad264ee3d8 --- /dev/null +++ b/src/test/ui/auxiliary/inference_unstable_itertools.rs @@ -0,0 +1,17 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub trait IpuItertools { + fn ipu_flatten(&self) -> u32 { + 1 + } +} + +impl IpuItertools for char {} diff --git a/src/test/ui/inference_unstable.rs b/src/test/ui/inference_unstable.rs new file mode 100644 index 00000000000..525fda33955 --- /dev/null +++ b/src/test/ui/inference_unstable.rs @@ -0,0 +1,29 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Ensures #[unstable] functions without opting in the corresponding #![feature] +// will not break inference. + +// aux-build:inference_unstable_iterator.rs +// aux-build:inference_unstable_itertools.rs +// run-pass + +extern crate inference_unstable_iterator; +extern crate inference_unstable_itertools; + +#[allow(unused_imports)] +use inference_unstable_iterator::IpuIterator; +use inference_unstable_itertools::IpuItertools; + +fn main() { + assert_eq!('x'.ipu_flatten(), 1); + //~^ WARN a method with this name will be added to the standard library in the future + //~^^ WARN it will become a hard error in a future release +} diff --git a/src/test/ui/inference_unstable.stderr b/src/test/ui/inference_unstable.stderr new file mode 100644 index 00000000000..a5cf4d6dff3 --- /dev/null +++ b/src/test/ui/inference_unstable.stderr @@ -0,0 +1,12 @@ +warning: a method with this name will be added to the standard library in the future + --> $DIR/inference_unstable.rs:26:20 + | +LL | assert_eq!('x'.ipu_flatten(), 1); + | ^^^^^^^^^^^ + | + = note: #[warn(unstable_name_collision)] 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 pr #48552 + = help: call with fully qualified syntax `inference_unstable_itertools::IpuItertools::ipu_flatten(...)` to keep using the current method + = note: add #![feature(ipu_flatten)] to the crate attributes to enable `inference_unstable_iterator::IpuIterator::ipu_flatten` + diff --git a/src/test/ui/inference_unstable_featured.rs b/src/test/ui/inference_unstable_featured.rs new file mode 100644 index 00000000000..f5c49bedc71 --- /dev/null +++ b/src/test/ui/inference_unstable_featured.rs @@ -0,0 +1,27 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// There should be E0034 "multiple applicable items in scope" if we opt-in for +// the feature. + +// aux-build:inference_unstable_iterator.rs +// aux-build:inference_unstable_itertools.rs + +#![feature(ipu_flatten)] + +extern crate inference_unstable_iterator; +extern crate inference_unstable_itertools; + +use inference_unstable_iterator::IpuIterator; +use inference_unstable_itertools::IpuItertools; + +fn main() { + assert_eq!('x'.ipu_flatten(), 0); //~ ERROR E0034 +} diff --git a/src/test/ui/inference_unstable_featured.stderr b/src/test/ui/inference_unstable_featured.stderr new file mode 100644 index 00000000000..cb5f3623291 --- /dev/null +++ b/src/test/ui/inference_unstable_featured.stderr @@ -0,0 +1,12 @@ +error[E0034]: multiple applicable items in scope + --> $DIR/inference_unstable_featured.rs:26:20 + | +LL | assert_eq!('x'.ipu_flatten(), 0); //~ ERROR E0034 + | ^^^^^^^^^^^ multiple `ipu_flatten` found + | + = note: candidate #1 is defined in an impl of the trait `inference_unstable_iterator::IpuIterator` for the type `char` + = note: candidate #2 is defined in an impl of the trait `inference_unstable_itertools::IpuItertools` for the type `char` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0034`. diff --git a/src/test/ui/inference_unstable_forced.rs b/src/test/ui/inference_unstable_forced.rs new file mode 100644 index 00000000000..82ce4034ce2 --- /dev/null +++ b/src/test/ui/inference_unstable_forced.rs @@ -0,0 +1,22 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// If the unstable API is the only possible solution, +// still emit E0658 "use of unstable library feature". + +// aux-build:inference_unstable_iterator.rs + +extern crate inference_unstable_iterator; + +use inference_unstable_iterator::IpuIterator; + +fn main() { + assert_eq!('x'.ipu_flatten(), 0); //~ ERROR E0658 +} diff --git a/src/test/ui/inference_unstable_forced.stderr b/src/test/ui/inference_unstable_forced.stderr new file mode 100644 index 00000000000..00eb81cd9a2 --- /dev/null +++ b/src/test/ui/inference_unstable_forced.stderr @@ -0,0 +1,11 @@ +error[E0658]: use of unstable library feature 'ipu_flatten' (see issue #99999) + --> $DIR/inference_unstable_forced.rs:21:20 + | +LL | assert_eq!('x'.ipu_flatten(), 0); //~ ERROR E0658 + | ^^^^^^^^^^^ + | + = help: add #![feature(ipu_flatten)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. -- 2.44.0