From c7168d306f3e95ce522ed8589ccb322a76e7a8d2 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Sun, 23 Jul 2017 22:30:47 +0900 Subject: [PATCH 01/17] Add hints when intercrate ambiguity causes overlap. --- src/librustc/traits/coherence.rs | 15 +++++-- src/librustc/traits/mod.rs | 5 +-- src/librustc/traits/select.rs | 29 ++++++++++++ src/librustc/traits/specialize/mod.rs | 16 +++++++ .../traits/specialize/specialization_graph.rs | 7 +-- .../coherence/inherent_impls_overlap.rs | 45 +++++++++++++------ 6 files changed, 95 insertions(+), 22 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index 34df447a11e15..dee81b8c4fa28 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -13,6 +13,7 @@ use hir::def_id::{DefId, LOCAL_CRATE}; use syntax_pos::DUMMY_SP; use traits::{self, Normalized, SelectionContext, Obligation, ObligationCause, Reveal}; +use traits::select::IntercrateAmbiguityCause; use ty::{self, Ty, TyCtxt}; use ty::subst::Subst; @@ -21,12 +22,17 @@ use infer::{InferCtxt, InferOk}; #[derive(Copy, Clone)] struct InferIsLocal(bool); +pub struct OverlapResult<'tcx> { + pub impl_header: ty::ImplHeader<'tcx>, + pub intercrate_ambiguity_causes: Vec, +} + /// If there are types that satisfy both impls, returns a suitably-freshened /// `ImplHeader` with those types substituted pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>, impl1_def_id: DefId, impl2_def_id: DefId) - -> Option> + -> Option> { debug!("impl_can_satisfy(\ impl1_def_id={:?}, \ @@ -65,7 +71,7 @@ fn with_fresh_ty_vars<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, ' fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, a_def_id: DefId, b_def_id: DefId) - -> Option> + -> Option> { debug!("overlap(a_def_id={:?}, b_def_id={:?})", a_def_id, @@ -113,7 +119,10 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>, return None } - Some(selcx.infcx().resolve_type_vars_if_possible(&a_impl_header)) + Some(OverlapResult { + impl_header: selcx.infcx().resolve_type_vars_if_possible(&a_impl_header), + intercrate_ambiguity_causes: selcx.intercrate_ambiguity_causes().to_vec(), + }) } pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index e14203b34a180..1283c5178b86e 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -28,9 +28,7 @@ use std::rc::Rc; use syntax::ast; use syntax_pos::{Span, DUMMY_SP}; -pub use self::coherence::orphan_check; -pub use self::coherence::overlapping_impls; -pub use self::coherence::OrphanCheckErr; +pub use self::coherence::{orphan_check, overlapping_impls, OrphanCheckErr, OverlapResult}; pub use self::fulfill::{FulfillmentContext, RegionObligation}; pub use self::project::MismatchedProjectionTypes; pub use self::project::{normalize, normalize_projection_type, Normalized}; @@ -38,6 +36,7 @@ pub use self::project::{ProjectionCache, ProjectionCacheSnapshot, Reveal}; pub use self::object_safety::ObjectSafetyViolation; pub use self::object_safety::MethodViolationCode; pub use self::select::{EvaluationCache, SelectionContext, SelectionCache}; +pub use self::select::IntercrateAmbiguityCause; pub use self::specialize::{OverlapError, specialization_graph, specializes, translate_substs}; pub use self::specialize::{SpecializesCache, find_associated_item}; pub use self::util::elaborate_predicates; diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index c690bebed8c00..d08203ae6d44a 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -90,6 +90,14 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { intercrate: bool, inferred_obligations: SnapshotVec>, + + intercrate_ambiguity_causes: Vec, +} + +#[derive(Clone)] +pub enum IntercrateAmbiguityCause { + DownstreamCrate(DefId), + UpstreamCrateUpdate(DefId), } // A stack that walks back up the stack frame. @@ -380,6 +388,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { freshener: infcx.freshener(), intercrate: false, inferred_obligations: SnapshotVec::new(), + intercrate_ambiguity_causes: Vec::new(), } } @@ -389,6 +398,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { freshener: infcx.freshener(), intercrate: true, inferred_obligations: SnapshotVec::new(), + intercrate_ambiguity_causes: Vec::new(), } } @@ -404,6 +414,10 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { self.infcx } + pub fn intercrate_ambiguity_causes(&self) -> &[IntercrateAmbiguityCause] { + &self.intercrate_ambiguity_causes + } + /// Wraps the inference context's in_snapshot s.t. snapshot handling is only from the selection /// context's self. fn in_snapshot(&mut self, f: F) -> R @@ -751,6 +765,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { if unbound_input_types && self.intercrate { debug!("evaluate_stack({:?}) --> unbound argument, intercrate --> ambiguous", stack.fresh_trait_ref); + // Heuristics: show the diagnostics when there are no candidates in crate. + if let Ok(candidate_set) = self.assemble_candidates(stack) { + if !candidate_set.ambiguous && candidate_set.vec.is_empty() { + let did = stack.fresh_trait_ref.def_id(); + self.intercrate_ambiguity_causes.push( + IntercrateAmbiguityCause::DownstreamCrate(did)); + } + } return EvaluatedToAmbig; } if unbound_input_types && @@ -1005,6 +1027,13 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { if !self.is_knowable(stack) { debug!("coherence stage: not knowable"); + // Heuristics: show the diagnostics when there are no candidates in crate. + let candidate_set = self.assemble_candidates(stack)?; + if !candidate_set.ambiguous && candidate_set.vec.is_empty() { + let did = stack.obligation.predicate.def_id(); + self.intercrate_ambiguity_causes.push( + IntercrateAmbiguityCause::UpstreamCrateUpdate(did)); + } return Ok(None); } diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 18734e2dbc3f1..5ae6fec000fd8 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -25,6 +25,7 @@ use hir::def_id::DefId; use infer::{InferCtxt, InferOk}; use ty::subst::{Subst, Substs}; use traits::{self, Reveal, ObligationCause}; +use traits::select::IntercrateAmbiguityCause; use ty::{self, TyCtxt, TypeFoldable}; use syntax_pos::DUMMY_SP; use std::rc::Rc; @@ -36,6 +37,7 @@ pub struct OverlapError { pub with_impl: DefId, pub trait_desc: String, pub self_desc: Option, + pub intercrate_ambiguity_causes: Vec, } /// Given a subst for the requested impl, translate it to a subst @@ -342,6 +344,20 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx } } + for cause in &overlap.intercrate_ambiguity_causes { + match cause { + &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { + err.note(&format!("downstream crates may implement {}", + tcx.item_path_str(def_id))); + } + &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { + err.note(&format!("upstream crates may add new impl for {} \ + in future versions", + tcx.item_path_str(def_id))); + } + } + } + err.emit(); } } else { diff --git a/src/librustc/traits/specialize/specialization_graph.rs b/src/librustc/traits/specialize/specialization_graph.rs index f80caeec460fa..e6ba13afbbb8a 100644 --- a/src/librustc/traits/specialize/specialization_graph.rs +++ b/src/librustc/traits/specialize/specialization_graph.rs @@ -113,7 +113,7 @@ impl<'a, 'gcx, 'tcx> Children { let overlap = traits::overlapping_impls(&infcx, possible_sibling, impl_def_id); - if let Some(impl_header) = overlap { + if let Some(overlap) = overlap { if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) { return Ok((false, false)); } @@ -123,7 +123,7 @@ impl<'a, 'gcx, 'tcx> Children { if le == ge { // overlap, but no specialization; error out - let trait_ref = impl_header.trait_ref.unwrap(); + let trait_ref = overlap.impl_header.trait_ref.unwrap(); let self_ty = trait_ref.self_ty(); Err(OverlapError { with_impl: possible_sibling, @@ -135,7 +135,8 @@ impl<'a, 'gcx, 'tcx> Children { Some(self_ty.to_string()) } else { None - } + }, + intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes, }) } else { Ok((le, ge)) diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 078ae34bc524c..07de54233041c 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -12,6 +12,7 @@ use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::traits; +use rustc::traits::IntercrateAmbiguityCause; use rustc::ty::{self, TyCtxt}; pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -26,7 +27,8 @@ struct InherentOverlapChecker<'a, 'tcx: 'a> { } impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { - fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) { + fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId, + overlap: traits::OverlapResult) { #[derive(Copy, Clone, PartialEq)] enum Namespace { Type, @@ -50,16 +52,32 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { for &item2 in &impl_items2[..] { if (name, namespace) == name_and_namespace(item2) { - struct_span_err!(self.tcx.sess, - self.tcx.span_of_impl(item1).unwrap(), - E0592, - "duplicate definitions with name `{}`", - name) - .span_label(self.tcx.span_of_impl(item1).unwrap(), - format!("duplicate definitions for `{}`", name)) - .span_label(self.tcx.span_of_impl(item2).unwrap(), - format!("other definition for `{}`", name)) - .emit(); + let mut err = struct_span_err!(self.tcx.sess, + self.tcx.span_of_impl(item1).unwrap(), + E0592, + "duplicate definitions with name `{}`", + name); + + err.span_label(self.tcx.span_of_impl(item1).unwrap(), + format!("duplicate definitions for `{}`", name)); + err.span_label(self.tcx.span_of_impl(item2).unwrap(), + format!("other definition for `{}`", name)); + + for cause in &overlap.intercrate_ambiguity_causes { + match cause { + &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { + err.note(&format!("downstream crates may implement {}", + self.tcx.item_path_str(def_id))); + } + &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { + err.note(&format!("upstream crates may add new impl for {} \ + in future versions", + self.tcx.item_path_str(def_id))); + } + } + } + + err.emit(); } } } @@ -71,8 +89,9 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { for (i, &impl1_def_id) in impls.iter().enumerate() { for &impl2_def_id in &impls[(i + 1)..] { self.tcx.infer_ctxt().enter(|infcx| { - if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() { - self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id) + if let Some(overlap) = + traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) { + self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap) } }); } From eeb16a1e0c626b55b00f5970bb917d91a1ca081d Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 25 Jul 2017 12:17:51 +0900 Subject: [PATCH 02/17] Slightly modify hint messages. --- src/librustc/traits/specialize/mod.rs | 4 ++-- src/librustc_typeck/coherence/inherent_impls_overlap.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 5ae6fec000fd8..c4b79003a1258 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -347,11 +347,11 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx for cause in &overlap.intercrate_ambiguity_causes { match cause { &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { - err.note(&format!("downstream crates may implement {}", + err.note(&format!("downstream crates may implement `{}`", tcx.item_path_str(def_id))); } &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { - err.note(&format!("upstream crates may add new impl for {} \ + err.note(&format!("upstream crates may add new impl for `{}` \ in future versions", tcx.item_path_str(def_id))); } diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 07de54233041c..e733b5328d3ba 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -66,11 +66,11 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { for cause in &overlap.intercrate_ambiguity_causes { match cause { &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { - err.note(&format!("downstream crates may implement {}", + err.note(&format!("downstream crates may implement `{}`", self.tcx.item_path_str(def_id))); } &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { - err.note(&format!("upstream crates may add new impl for {} \ + err.note(&format!("upstream crates may add new impl for `{}` \ in future versions", self.tcx.item_path_str(def_id))); } From c18c1d5b7bca7356a216438c1847d2a625707415 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 25 Jul 2017 15:46:26 +0900 Subject: [PATCH 03/17] Add tests for intercrate ambiguity hints. --- .../coherence-overlap-issue-23516-inherent.rs | 26 +++++++++++++++++ .../coherence-overlap-issue-23516.rs | 7 ++++- .../coherence-overlap-upstream-inherent.rs | 28 +++++++++++++++++++ .../coherence-overlap-upstream.rs | 28 +++++++++++++++++++ .../overlapping_inherent_impls.stderr | 1 + 5 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs create mode 100644 src/test/compile-fail/coherence-overlap-upstream-inherent.rs create mode 100644 src/test/compile-fail/coherence-overlap-upstream.rs diff --git a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs new file mode 100644 index 0000000000000..dfdcc1bc7cb43 --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs @@ -0,0 +1,26 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `Box: !Sugar` to be ambiguous, even +// though we see no impl of `Sugar` for `Box`. Therefore, an overlap +// error is reported for the following pair of impls (#23516). + +pub trait Sugar {} + +struct Cake(X); + +impl Cake { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +//~| NOTE upstream crates may add new impl for `Sugar` in future versions +impl Cake> { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-issue-23516.rs b/src/test/compile-fail/coherence-overlap-issue-23516.rs index 51d7c3e8b4cb1..ffde4011ee4d8 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516.rs @@ -15,5 +15,10 @@ pub trait Sugar { fn dummy(&self) { } } pub trait Sweet { fn dummy(&self) { } } impl Sweet for T { } -impl Sweet for Box { } //~ ERROR E0119 +//~^ NOTE first implementation here +impl Sweet for Box { } +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `std::boxed::Box<_>` +//~| NOTE upstream crates may add new impl for `Sugar` in future versions + fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-upstream-inherent.rs b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs new file mode 100644 index 0000000000000..2ec51b1bbe33e --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs @@ -0,0 +1,28 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `i16: Remote` to be ambiguous, even +// though the upstream crate doesn't implement it for now. + +// aux-build:coherence_lib.rs + +extern crate coherence_lib; + +use coherence_lib::Remote; + +struct A(X); +impl A where T: Remote { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +//~| NOTE upstream crates may add new impl for `coherence_lib::Remote` in future versions +impl A { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +fn main() {} diff --git a/src/test/compile-fail/coherence-overlap-upstream.rs b/src/test/compile-fail/coherence-overlap-upstream.rs new file mode 100644 index 0000000000000..81c22e0685052 --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-upstream.rs @@ -0,0 +1,28 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Tests that we consider `i16: Remote` to be ambiguous, even +// though the upstream crate doesn't implement it for now. + +// aux-build:coherence_lib.rs + +extern crate coherence_lib; + +use coherence_lib::Remote; + +trait Foo {} +impl Foo for T where T: Remote {} +//~^ NOTE first implementation here +impl Foo for i16 {} +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `i16` +//~| NOTE upstream crates may add new impl for `coherence_lib::Remote` in future versions + +fn main() {} diff --git a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr index de8a24cf33f44..186bccdd25407 100644 --- a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr +++ b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr @@ -24,6 +24,7 @@ error[E0592]: duplicate definitions with name `baz` ... 43 | fn baz(&self) {} | ---------------- other definition for `baz` + = note: upstream crates may add new impl for `std::marker::Copy` in future versions error: aborting due to 3 previous errors From 51e92bb00b03b2e14fef74478bbb02bbc6b554c4 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 25 Jul 2017 16:50:49 +0900 Subject: [PATCH 04/17] Fix a very subtle mistake in a ui test. --- src/test/ui/codemap_tests/overlapping_inherent_impls.stderr | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr index 186bccdd25407..f6ca4f15f9ed8 100644 --- a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr +++ b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr @@ -24,6 +24,7 @@ error[E0592]: duplicate definitions with name `baz` ... 43 | fn baz(&self) {} | ---------------- other definition for `baz` + | = note: upstream crates may add new impl for `std::marker::Copy` in future versions error: aborting due to 3 previous errors From 2cc72866eadf7928aab42c4f2ec527d7dcd17679 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 25 Jul 2017 16:52:36 +0900 Subject: [PATCH 05/17] Unify intercrate ambiguity emitters into a function. --- src/librustc/traits/select.rs | 20 +++++++++++++++++++ src/librustc/traits/specialize/mod.rs | 12 +---------- .../coherence/inherent_impls_overlap.rs | 13 +----------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index d08203ae6d44a..9aa95233e4718 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -100,6 +100,26 @@ pub enum IntercrateAmbiguityCause { UpstreamCrateUpdate(DefId), } +impl IntercrateAmbiguityCause { + /// Emits notes when the overlap is caused by complex intercrate ambiguities. + /// See #23980 for details. + pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + err: &mut ::errors::DiagnosticBuilder) { + match self { + &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { + err.note(&format!("downstream crates may implement `{}`", + tcx.item_path_str(def_id))); + } + &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { + err.note(&format!("upstream crates may add new impl for `{}` \ + in future versions", + tcx.item_path_str(def_id))); + } + } + } +} + // A stack that walks back up the stack frame. struct TraitObligationStack<'prev, 'tcx: 'prev> { obligation: &'prev TraitObligation<'tcx>, diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index c4b79003a1258..88cbc03a4631d 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -345,17 +345,7 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx } for cause in &overlap.intercrate_ambiguity_causes { - match cause { - &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { - err.note(&format!("downstream crates may implement `{}`", - tcx.item_path_str(def_id))); - } - &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { - err.note(&format!("upstream crates may add new impl for `{}` \ - in future versions", - tcx.item_path_str(def_id))); - } - } + cause.add_intercrate_ambiguity_hint(tcx, &mut err); } err.emit(); diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index e733b5328d3ba..0fad16c958222 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -12,7 +12,6 @@ use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::traits; -use rustc::traits::IntercrateAmbiguityCause; use rustc::ty::{self, TyCtxt}; pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, @@ -64,17 +63,7 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { format!("other definition for `{}`", name)); for cause in &overlap.intercrate_ambiguity_causes { - match cause { - &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { - err.note(&format!("downstream crates may implement `{}`", - self.tcx.item_path_str(def_id))); - } - &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { - err.note(&format!("upstream crates may add new impl for `{}` \ - in future versions", - self.tcx.item_path_str(def_id))); - } - } + cause.add_intercrate_ambiguity_hint(self.tcx, &mut err); } err.emit(); From 24abea97c7130ac2031f5105dae42e3eb0a867ab Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 1 Aug 2017 10:27:25 +0430 Subject: [PATCH 06/17] Print more detailed trait-ref for intercrate ambiguity. --- src/librustc/traits/select.rs | 59 ++++++++++++++----- src/librustc/traits/specialize/mod.rs | 2 +- .../coherence/inherent_impls_overlap.rs | 2 +- .../coherence-overlap-issue-23516-inherent.rs | 2 +- .../coherence-overlap-issue-23516.rs | 2 +- .../coherence-overlap-upstream-inherent.rs | 2 +- .../coherence-overlap-upstream.rs | 2 +- .../overlapping_inherent_impls.stderr | 2 +- 8 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 9aa95233e4718..6e51f04146e27 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -96,25 +96,36 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> { #[derive(Clone)] pub enum IntercrateAmbiguityCause { - DownstreamCrate(DefId), - UpstreamCrateUpdate(DefId), + DownstreamCrate { + trait_desc: String, + self_desc: Option, + }, + UpstreamCrateUpdate { + trait_desc: String, + self_desc: Option, + }, } impl IntercrateAmbiguityCause { /// Emits notes when the overlap is caused by complex intercrate ambiguities. /// See #23980 for details. pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self, - tcx: TyCtxt<'a, 'tcx, 'tcx>, err: &mut ::errors::DiagnosticBuilder) { match self { - &IntercrateAmbiguityCause::DownstreamCrate(def_id) => { - err.note(&format!("downstream crates may implement `{}`", - tcx.item_path_str(def_id))); - } - &IntercrateAmbiguityCause::UpstreamCrateUpdate(def_id) => { - err.note(&format!("upstream crates may add new impl for `{}` \ + &IntercrateAmbiguityCause::DownstreamCrate { ref trait_desc, ref self_desc } => { + let self_desc = if let &Some(ref ty) = self_desc { + format!(" for type `{}`", ty) + } else { "".to_string() }; + err.note(&format!("downstream crates may implement trait `{}`{}", + trait_desc, self_desc)); + } + &IntercrateAmbiguityCause::UpstreamCrateUpdate { ref trait_desc, ref self_desc } => { + let self_desc = if let &Some(ref ty) = self_desc { + format!(" for type `{}`", ty) + } else { "".to_string() }; + err.note(&format!("upstream crates may add new impl of trait `{}`{} \ in future versions", - tcx.item_path_str(def_id))); + trait_desc, self_desc)); } } } @@ -788,9 +799,17 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // Heuristics: show the diagnostics when there are no candidates in crate. if let Ok(candidate_set) = self.assemble_candidates(stack) { if !candidate_set.ambiguous && candidate_set.vec.is_empty() { - let did = stack.fresh_trait_ref.def_id(); - self.intercrate_ambiguity_causes.push( - IntercrateAmbiguityCause::DownstreamCrate(did)); + let trait_ref = stack.obligation.predicate.skip_binder().trait_ref; + let self_ty = trait_ref.self_ty(); + let cause = IntercrateAmbiguityCause::DownstreamCrate { + trait_desc: trait_ref.to_string(), + self_desc: if self_ty.has_concrete_skeleton() { + Some(self_ty.to_string()) + } else { + None + }, + }; + self.intercrate_ambiguity_causes.push(cause); } } return EvaluatedToAmbig; @@ -1050,9 +1069,17 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { // Heuristics: show the diagnostics when there are no candidates in crate. let candidate_set = self.assemble_candidates(stack)?; if !candidate_set.ambiguous && candidate_set.vec.is_empty() { - let did = stack.obligation.predicate.def_id(); - self.intercrate_ambiguity_causes.push( - IntercrateAmbiguityCause::UpstreamCrateUpdate(did)); + let trait_ref = stack.obligation.predicate.skip_binder().trait_ref; + let self_ty = trait_ref.self_ty(); + let cause = IntercrateAmbiguityCause::UpstreamCrateUpdate { + trait_desc: trait_ref.to_string(), + self_desc: if self_ty.has_concrete_skeleton() { + Some(self_ty.to_string()) + } else { + None + }, + }; + self.intercrate_ambiguity_causes.push(cause); } return Ok(None); } diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 88cbc03a4631d..7dbf37decd594 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -345,7 +345,7 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx } for cause in &overlap.intercrate_ambiguity_causes { - cause.add_intercrate_ambiguity_hint(tcx, &mut err); + cause.add_intercrate_ambiguity_hint(&mut err); } err.emit(); diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs index 0fad16c958222..76dcfe36e4fcd 100644 --- a/src/librustc_typeck/coherence/inherent_impls_overlap.rs +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -63,7 +63,7 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { format!("other definition for `{}`", name)); for cause in &overlap.intercrate_ambiguity_causes { - cause.add_intercrate_ambiguity_hint(self.tcx, &mut err); + cause.add_intercrate_ambiguity_hint(&mut err); } err.emit(); diff --git a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs index dfdcc1bc7cb43..9b8ad51c5ff7d 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs @@ -19,7 +19,7 @@ struct Cake(X); impl Cake { fn dummy(&self) { } } //~^ ERROR E0592 //~| NOTE duplicate definitions for `dummy` -//~| NOTE upstream crates may add new impl for `Sugar` in future versions +//~| NOTE upstream crates may add new impl of trait `Sugar` for type `std::boxed::Box<_>` impl Cake> { fn dummy(&self) { } } //~^ NOTE other definition for `dummy` diff --git a/src/test/compile-fail/coherence-overlap-issue-23516.rs b/src/test/compile-fail/coherence-overlap-issue-23516.rs index ffde4011ee4d8..950d1fe29bb91 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516.rs @@ -19,6 +19,6 @@ impl Sweet for T { } impl Sweet for Box { } //~^ ERROR E0119 //~| NOTE conflicting implementation for `std::boxed::Box<_>` -//~| NOTE upstream crates may add new impl for `Sugar` in future versions +//~| NOTE upstream crates may add new impl of trait `Sugar` for type `std::boxed::Box<_>` fn main() { } diff --git a/src/test/compile-fail/coherence-overlap-upstream-inherent.rs b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs index 2ec51b1bbe33e..1d0c63110cecd 100644 --- a/src/test/compile-fail/coherence-overlap-upstream-inherent.rs +++ b/src/test/compile-fail/coherence-overlap-upstream-inherent.rs @@ -21,7 +21,7 @@ struct A(X); impl A where T: Remote { fn dummy(&self) { } } //~^ ERROR E0592 //~| NOTE duplicate definitions for `dummy` -//~| NOTE upstream crates may add new impl for `coherence_lib::Remote` in future versions +//~| NOTE upstream crates may add new impl of trait `coherence_lib::Remote` for type `i16` impl A { fn dummy(&self) { } } //~^ NOTE other definition for `dummy` diff --git a/src/test/compile-fail/coherence-overlap-upstream.rs b/src/test/compile-fail/coherence-overlap-upstream.rs index 81c22e0685052..e978143a067c5 100644 --- a/src/test/compile-fail/coherence-overlap-upstream.rs +++ b/src/test/compile-fail/coherence-overlap-upstream.rs @@ -23,6 +23,6 @@ impl Foo for T where T: Remote {} impl Foo for i16 {} //~^ ERROR E0119 //~| NOTE conflicting implementation for `i16` -//~| NOTE upstream crates may add new impl for `coherence_lib::Remote` in future versions +//~| NOTE upstream crates may add new impl of trait `coherence_lib::Remote` for type `i16` fn main() {} diff --git a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr index f6ca4f15f9ed8..eaf42cde22f76 100644 --- a/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr +++ b/src/test/ui/codemap_tests/overlapping_inherent_impls.stderr @@ -25,7 +25,7 @@ error[E0592]: duplicate definitions with name `baz` 43 | fn baz(&self) {} | ---------------- other definition for `baz` | - = note: upstream crates may add new impl for `std::marker::Copy` in future versions + = note: upstream crates may add new impl of trait `std::marker::Copy` for type `std::vec::Vec<_>` in future versions error: aborting due to 3 previous errors From ca684a6476df7f5524fecedef19f3820925d977b Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 1 Aug 2017 11:23:02 +0430 Subject: [PATCH 07/17] Add downstream tests for intercrate ambiguity hints. --- .../coherence-overlap-downstream-inherent.rs | 32 +++++++++++++++++++ .../coherence-overlap-downstream.rs | 32 +++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 src/test/compile-fail/coherence-overlap-downstream-inherent.rs create mode 100644 src/test/compile-fail/coherence-overlap-downstream.rs diff --git a/src/test/compile-fail/coherence-overlap-downstream-inherent.rs b/src/test/compile-fail/coherence-overlap-downstream-inherent.rs new file mode 100644 index 0000000000000..66068b535556c --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-downstream-inherent.rs @@ -0,0 +1,32 @@ +// Copyright 2017 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. + +// Tests that we consider `T: Sugar + Fruit` to be ambiguous, even +// though no impls are found. + +struct Sweet(X); +pub trait Sugar {} +pub trait Fruit {} +impl Sweet { fn dummy(&self) { } } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `dummy` +impl Sweet { fn dummy(&self) { } } +//~^ NOTE other definition for `dummy` + +trait Bar {} +struct A(T, X); +impl A where T: Bar { fn f(&self) {} } +//~^ ERROR E0592 +//~| NOTE duplicate definitions for `f` +//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32` +impl A { fn f(&self) {} } +//~^ NOTE other definition for `f` + +fn main() {} diff --git a/src/test/compile-fail/coherence-overlap-downstream.rs b/src/test/compile-fail/coherence-overlap-downstream.rs new file mode 100644 index 0000000000000..1df02737dec58 --- /dev/null +++ b/src/test/compile-fail/coherence-overlap-downstream.rs @@ -0,0 +1,32 @@ +// Copyright 2017 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. + +// Tests that we consider `T: Sugar + Fruit` to be ambiguous, even +// though no impls are found. + +pub trait Sugar {} +pub trait Fruit {} +pub trait Sweet {} +impl Sweet for T { } +//~^ NOTE first implementation here +impl Sweet for T { } +//~^ ERROR E0119 +//~| NOTE conflicting implementation + +pub trait Foo {} +pub trait Bar {} +impl Foo for T where T: Bar {} +//~^ NOTE first implementation here +impl Foo for i32 {} +//~^ ERROR E0119 +//~| NOTE conflicting implementation for `i32` +//~| NOTE downstream crates may implement trait `Bar<_>` for type `i32` + +fn main() { } From 59cff34e94cc038a0e07a06b606142c1f01b0708 Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Tue, 1 Aug 2017 12:37:11 +0430 Subject: [PATCH 08/17] Fix misdetection of upstream intercrate ambiguity. --- src/librustc/traits/select.rs | 21 ++++++++++++------- .../coherence-overlap-issue-23516-inherent.rs | 2 +- .../coherence-overlap-issue-23516.rs | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 6e51f04146e27..a5a95209be035 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -31,7 +31,7 @@ use super::{VtableImplData, VtableObjectData, VtableBuiltinData, use super::util; use dep_graph::{DepNodeIndex, DepKind}; -use hir::def_id::DefId; +use hir::def_id::{DefId, LOCAL_CRATE}; use infer; use infer::{InferCtxt, InferOk, TypeFreshener}; use ty::subst::{Kind, Subst, Substs}; @@ -1071,13 +1071,18 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { if !candidate_set.ambiguous && candidate_set.vec.is_empty() { let trait_ref = stack.obligation.predicate.skip_binder().trait_ref; let self_ty = trait_ref.self_ty(); - let cause = IntercrateAmbiguityCause::UpstreamCrateUpdate { - trait_desc: trait_ref.to_string(), - self_desc: if self_ty.has_concrete_skeleton() { - Some(self_ty.to_string()) - } else { - None - }, + let trait_desc = trait_ref.to_string(); + let self_desc = if self_ty.has_concrete_skeleton() { + Some(self_ty.to_string()) + } else { + None + }; + let cause = if + trait_ref.def_id.krate != LOCAL_CRATE && + !self.tcx().has_attr(trait_ref.def_id, "fundamental") { + IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc } + } else { + IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc } }; self.intercrate_ambiguity_causes.push(cause); } diff --git a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs index 9b8ad51c5ff7d..355af60710a9b 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516-inherent.rs @@ -19,7 +19,7 @@ struct Cake(X); impl Cake { fn dummy(&self) { } } //~^ ERROR E0592 //~| NOTE duplicate definitions for `dummy` -//~| NOTE upstream crates may add new impl of trait `Sugar` for type `std::boxed::Box<_>` +//~| NOTE downstream crates may implement trait `Sugar` for type `std::boxed::Box<_>` impl Cake> { fn dummy(&self) { } } //~^ NOTE other definition for `dummy` diff --git a/src/test/compile-fail/coherence-overlap-issue-23516.rs b/src/test/compile-fail/coherence-overlap-issue-23516.rs index 950d1fe29bb91..ffef5bf10871a 100644 --- a/src/test/compile-fail/coherence-overlap-issue-23516.rs +++ b/src/test/compile-fail/coherence-overlap-issue-23516.rs @@ -19,6 +19,6 @@ impl Sweet for T { } impl Sweet for Box { } //~^ ERROR E0119 //~| NOTE conflicting implementation for `std::boxed::Box<_>` -//~| NOTE upstream crates may add new impl of trait `Sugar` for type `std::boxed::Box<_>` +//~| NOTE downstream crates may implement trait `Sugar` for type `std::boxed::Box<_>` fn main() { } From 6fc35de5e88bb4035fa2a2be28b09d01fc8ebd53 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Sun, 27 Aug 2017 19:20:03 +0200 Subject: [PATCH 09/17] Fail ./x.py on invalid command Make the ./x.py script fail when run with an invalid command, like: ./x.py nonsense This helps in case of chaining multiple runs, eg.: ./x.py biuld && ./x.py test --- src/bootstrap/flags.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index a84d43d3deede..7546d7fd4f07a 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -136,7 +136,7 @@ To learn more about a subcommand, run `./x.py -h`"); None => { // No subcommand -- show the general usage and subcommand help println!("{}\n", subcommand_help); - process::exit(0); + process::exit(1); } }; From 45d31ac2f2df65777acd7e02f195efdb03b4d9ef Mon Sep 17 00:00:00 2001 From: Tatsuyuki Ishi Date: Mon, 28 Aug 2017 12:52:02 +0900 Subject: [PATCH 10/17] bootstrap: remove unneeded extern crate The crate itself is internally referenced by serde_derive. --- src/bootstrap/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 55358f2ffcb73..84a9e56b644c8 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -123,7 +123,6 @@ extern crate build_helper; extern crate serde_derive; #[macro_use] extern crate lazy_static; -extern crate serde; extern crate serde_json; extern crate cmake; extern crate filetime; From cc5ea04e1f384d33d818a084b05ea46fba043150 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Sun, 27 Aug 2017 20:41:12 -0700 Subject: [PATCH 11/17] un-regress behavior of `unused_results` lint for booleans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This, as #43813, is due to the author of #43728 (specifically, 3645b062) being a damnably contemptible fool. Before this entire fiasco, we would return early from the unusedness late lints pass if the type of the expression within the `hir::StmtSemi` was `!`, `()`, or a boolean: these types would never get to the point of being marked as unused results. That is, until the dunce who somehow (!?) came to be trusted with the plum responsibility of implementing RFC 1940 (`#[must_use]` for functions) went and fouled everything up, removing the early returns based on the (stupid) thought that there would be no harm in it, since we would need to continue to check these types being returned from must_use functions (which was true for the booleans, at least). But there was harm—harm that any quarter-way-competent programmer would have surely forseen! For after the new functional-must-use checks, there was nothing to stop the previously-returned-early types from falling through to be marked by the unused-results lint!—a monumentally idiotic error that has cost the project tens of precious developer- and reviewer-minutes dealing with the fallout here and in #43813. If 3645b062 is representative of the standard of craftsmanship the rising generation of software engineers holds themselves to, I weep for the future of our technological civilization. Resolves #44119. --- src/librustc_lint/unused.rs | 9 ++++++++- src/test/compile-fail/unused-result.rs | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index cbc4ebe90fd09..e8de6c36ba18d 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -182,7 +182,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedResults { } if !(ty_warned || fn_warned) { - cx.span_lint(UNUSED_RESULTS, s.span, "unused result"); + match t.sty { + // Historically, booleans have not been considered unused + // results. (See Issue #44119.) + ty::TyBool => return, + _ => { + cx.span_lint(UNUSED_RESULTS, s.span, "unused result"); + } + } } fn check_must_use(cx: &LateContext, def_id: DefId, sp: Span, describe_path: &str) -> bool { diff --git a/src/test/compile-fail/unused-result.rs b/src/test/compile-fail/unused-result.rs index 0c6c7fc5a0d75..c0417e6e997ca 100644 --- a/src/test/compile-fail/unused-result.rs +++ b/src/test/compile-fail/unused-result.rs @@ -42,6 +42,9 @@ fn main() { foo::(); //~ ERROR: unused `MustUse` which must be used foo::(); //~ ERROR: unused `MustUseMsg` which must be used: some message + // as an exceptional case, booleans are not considered unused + foo::(); + let _ = foo::(); let _ = foo::(); let _ = foo::(); From 2bffa310a8e55c3278492c42cc33d88e16cb0cbf Mon Sep 17 00:00:00 2001 From: Thomas Jespersen Date: Mon, 28 Aug 2017 13:35:53 +0200 Subject: [PATCH 12/17] compiletest: Change Config comments to doc comments --- src/tools/compiletest/src/common.rs | 78 ++++++++++++++--------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 0d6b350a1d431..cee7e52c7f3c6 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -83,117 +83,117 @@ impl fmt::Display for Mode { #[derive(Clone)] pub struct Config { - // The library paths required for running the compiler + /// The library paths required for running the compiler pub compile_lib_path: PathBuf, - // The library paths required for running compiled programs + /// The library paths required for running compiled programs pub run_lib_path: PathBuf, - // The rustc executable + /// The rustc executable pub rustc_path: PathBuf, - // The rustdoc executable + /// The rustdoc executable pub rustdoc_path: Option, - // The python executable to use for LLDB + /// The python executable to use for LLDB pub lldb_python: String, - // The python executable to use for htmldocck + /// The python executable to use for htmldocck pub docck_python: String, - // The llvm FileCheck binary path + /// The llvm FileCheck binary path pub llvm_filecheck: Option, - // The valgrind path + /// The valgrind path pub valgrind_path: Option, - // Whether to fail if we can't run run-pass-valgrind tests under valgrind - // (or, alternatively, to silently run them like regular run-pass tests). + /// Whether to fail if we can't run run-pass-valgrind tests under valgrind + /// (or, alternatively, to silently run them like regular run-pass tests). pub force_valgrind: bool, - // The directory containing the tests to run + /// The directory containing the tests to run pub src_base: PathBuf, - // The directory where programs should be built + /// The directory where programs should be built pub build_base: PathBuf, - // The name of the stage being built (stage1, etc) + /// The name of the stage being built (stage1, etc) pub stage_id: String, - // The test mode, compile-fail, run-fail, run-pass + /// The test mode, compile-fail, run-fail, run-pass pub mode: Mode, - // Run ignored tests + /// Run ignored tests pub run_ignored: bool, - // Only run tests that match this filter + /// Only run tests that match this filter pub filter: Option, - // Exactly match the filter, rather than a substring + /// Exactly match the filter, rather than a substring pub filter_exact: bool, - // Write out a parseable log of tests that were run + /// Write out a parseable log of tests that were run pub logfile: Option, - // A command line to prefix program execution with, - // for running under valgrind + /// A command line to prefix program execution with, + /// for running under valgrind pub runtool: Option, - // Flags to pass to the compiler when building for the host + /// Flags to pass to the compiler when building for the host pub host_rustcflags: Option, - // Flags to pass to the compiler when building for the target + /// Flags to pass to the compiler when building for the target pub target_rustcflags: Option, - // Target system to be tested + /// Target system to be tested pub target: String, - // Host triple for the compiler being invoked + /// Host triple for the compiler being invoked pub host: String, - // Path to / name of the GDB executable + /// Path to / name of the GDB executable pub gdb: Option, - // Version of GDB, encoded as ((major * 1000) + minor) * 1000 + patch + /// Version of GDB, encoded as ((major * 1000) + minor) * 1000 + patch pub gdb_version: Option, - // Whether GDB has native rust support + /// Whether GDB has native rust support pub gdb_native_rust: bool, - // Version of LLDB + /// Version of LLDB pub lldb_version: Option, - // Version of LLVM + /// Version of LLVM pub llvm_version: Option, - // Is LLVM a system LLVM + /// Is LLVM a system LLVM pub system_llvm: bool, - // Path to the android tools + /// Path to the android tools pub android_cross_path: PathBuf, - // Extra parameter to run adb on arm-linux-androideabi + /// Extra parameter to run adb on arm-linux-androideabi pub adb_path: String, - // Extra parameter to run test suite on arm-linux-androideabi + /// Extra parameter to run test suite on arm-linux-androideabi pub adb_test_dir: String, - // status whether android device available or not + /// status whether android device available or not pub adb_device_status: bool, - // the path containing LLDB's Python module + /// the path containing LLDB's Python module pub lldb_python_dir: Option, - // Explain what's going on + /// Explain what's going on pub verbose: bool, - // Print one character per test instead of one line + /// Print one character per test instead of one line pub quiet: bool, - // Whether to use colors in test. + /// Whether to use colors in test. pub color: ColorConfig, - // where to find the remote test client process, if we're using it + /// where to find the remote test client process, if we're using it pub remote_test_client: Option, // Configuration for various run-make tests frobbing things like C compilers From 10bd39e7af94910e9cdc57e63c966d2020223d8f Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sun, 27 Aug 2017 12:17:07 -0700 Subject: [PATCH 13/17] Rewrite `std::net::ToSocketAddrs` doc examples. in particular: * show how to create an iterator that yields multiple socket addresses * show more failing scenarios --- src/libstd/net/addr.rs | 88 +++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/src/libstd/net/addr.rs b/src/libstd/net/addr.rs index 36c06dc0b58d0..9ef19cd64b386 100644 --- a/src/libstd/net/addr.rs +++ b/src/libstd/net/addr.rs @@ -705,30 +705,74 @@ impl hash::Hash for SocketAddrV6 { /// /// # Examples /// +/// Creating a [`SocketAddr`] iterator that yields one item: +/// +/// ``` +/// use std::net::{ToSocketAddrs, SocketAddr}; +/// +/// let addr = SocketAddr::from(([127, 0, 0, 1], 443)); +/// let mut addrs_iter = addr.to_socket_addrs().unwrap(); +/// +/// assert_eq!(Some(addr), addrs_iter.next()); +/// assert!(addrs_iter.next().is_none()); +/// ``` +/// +/// Creating a [`SocketAddr`] iterator from a hostname: +/// /// ```no_run -/// use std::net::{SocketAddrV4, TcpStream, UdpSocket, TcpListener, Ipv4Addr}; -/// -/// fn main() { -/// let ip = Ipv4Addr::new(127, 0, 0, 1); -/// let port = 12345; -/// -/// // The following lines are equivalent modulo possible "localhost" name -/// // resolution differences -/// let tcp_s = TcpStream::connect(SocketAddrV4::new(ip, port)); -/// let tcp_s = TcpStream::connect((ip, port)); -/// let tcp_s = TcpStream::connect(("127.0.0.1", port)); -/// let tcp_s = TcpStream::connect(("localhost", port)); -/// let tcp_s = TcpStream::connect("127.0.0.1:12345"); -/// let tcp_s = TcpStream::connect("localhost:12345"); -/// -/// // TcpListener::bind(), UdpSocket::bind() and UdpSocket::send_to() -/// // behave similarly -/// let tcp_l = TcpListener::bind("localhost:12345"); -/// -/// let mut udp_s = UdpSocket::bind(("127.0.0.1", port)).unwrap(); -/// udp_s.send_to(&[7], (ip, 23451)).unwrap(); -/// } +/// use std::net::{SocketAddr, ToSocketAddrs}; +/// +/// // assuming 'localhost' resolves to 127.0.0.1 +/// let mut addrs_iter = "localhost:443".to_socket_addrs().unwrap(); +/// assert_eq!(addrs_iter.next(), Some(SocketAddr::from(([127, 0, 0, 1], 443)))); +/// assert!(addrs_iter.next().is_none()); +/// +/// // assuming 'foo' does not resolve +/// assert!("foo:443".to_socket_addrs().is_err()); /// ``` +/// +/// Creating a [`SocketAddr`] iterator that yields multiple items: +/// +/// ``` +/// use std::net::{SocketAddr, ToSocketAddrs}; +/// +/// let addr1 = SocketAddr::from(([0, 0, 0, 0], 80)); +/// let addr2 = SocketAddr::from(([127, 0, 0, 1], 443)); +/// let addrs = vec![addr1, addr2]; +/// +/// let mut addrs_iter = (&addrs[..]).to_socket_addrs().unwrap(); +/// +/// assert_eq!(Some(addr1), addrs_iter.next()); +/// assert_eq!(Some(addr2), addrs_iter.next()); +/// assert!(addrs_iter.next().is_none()); +/// ``` +/// +/// Attempting to create a [`SocketAddr`] iterator from an improperly formatted +/// socket address `&str` (missing the port): +/// +/// ``` +/// use std::io; +/// use std::net::ToSocketAddrs; +/// +/// let err = "127.0.0.1".to_socket_addrs().unwrap_err(); +/// assert_eq!(err.kind(), io::ErrorKind::InvalidInput); +/// ``` +/// +/// [`TcpStream::connect`] is an example of an function that utilizes +/// `ToSocketsAddr` as a trait bound on its parameter in order to accept +/// different types: +/// +/// ```no_run +/// use std::net::{TcpStream, Ipv4Addr}; +/// +/// let stream = TcpStream::connect(("127.0.0.1", 443)); +/// // or +/// let stream = TcpStream::connect("127.0.0.1.443"); +/// // or +/// let stream = TcpStream::connect((Ipv4Addr::new(127, 0, 0, 1), 443)); +/// ``` +/// +/// [`TcpStream::connect`]: ../../std/net/struct.TcpStream.html#method.connect #[stable(feature = "rust1", since = "1.0.0")] pub trait ToSocketAddrs { /// Returned iterator over socket addresses which this type may correspond From f50bf8636e3b0296db82e631fe95c84324a46ccc Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 28 Aug 2017 22:40:09 +0200 Subject: [PATCH 14/17] Fix invalid linker position --- src/librustdoc/html/format.rs | 2 +- src/librustdoc/html/render.rs | 6 ++---- src/librustdoc/html/static/rustdoc.css | 7 ++++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 988890ffedcdd..10a3878073e97 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -228,7 +228,7 @@ impl<'a> fmt::Display for WhereClause<'a> { } if end_newline { - //add a space so stripping
tags and breaking spaces still renders properly + // add a space so stripping
tags and breaking spaces still renders properly if f.alternate() { clause.push(' '); } else { diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 5457f69cb6dab..5b8c7503a791d 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1523,8 +1523,7 @@ impl<'a> fmt::Display for Item<'a> { } else { write!(fmt, "Module ")?; }, - clean::FunctionItem(..) | clean::ForeignFunctionItem(..) => - write!(fmt, "Function ")?, + clean::FunctionItem(..) | clean::ForeignFunctionItem(..) => write!(fmt, "Function ")?, clean::TraitItem(..) => write!(fmt, "Trait ")?, clean::StructItem(..) => write!(fmt, "Struct ")?, clean::UnionItem(..) => write!(fmt, "Union ")?, @@ -1532,8 +1531,7 @@ impl<'a> fmt::Display for Item<'a> { clean::TypedefItem(..) => write!(fmt, "Type Definition ")?, clean::MacroItem(..) => write!(fmt, "Macro ")?, clean::PrimitiveItem(..) => write!(fmt, "Primitive Type ")?, - clean::StaticItem(..) | clean::ForeignStaticItem(..) => - write!(fmt, "Static ")?, + clean::StaticItem(..) | clean::ForeignStaticItem(..) => write!(fmt, "Static ")?, clean::ConstantItem(..) => write!(fmt, "Constant ")?, _ => { // We don't generate pages for any other type. diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 4a3286b421ae9..312dfce8d39c2 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -329,6 +329,10 @@ h4 > code, h3 > code, .invisible > code { display: inline-block; } +.in-band > code { + display: inline-block; +} + #main { position: relative; } #main > .since { top: inherit; @@ -447,7 +451,8 @@ a { } .in-band:hover > .anchor { - display: initial; + display: inline-block; + position: absolute; } .anchor { display: none; From 5c287406c23076cc72e2f6d34355d34a5f263d7a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 28 Aug 2017 16:50:41 -0400 Subject: [PATCH 15/17] factor out helper method --- src/librustc/traits/coherence.rs | 10 ++++++---- src/librustc/traits/select.rs | 5 ++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/librustc/traits/coherence.rs b/src/librustc/traits/coherence.rs index dee81b8c4fa28..edb6bcf4f8ccc 100644 --- a/src/librustc/traits/coherence.rs +++ b/src/librustc/traits/coherence.rs @@ -140,10 +140,7 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, // if the trait is not marked fundamental, then it's always possible that // an ancestor crate will impl this in the future, if they haven't // already - if - trait_ref.def_id.krate != LOCAL_CRATE && - !tcx.has_attr(trait_ref.def_id, "fundamental") - { + if !trait_ref_is_local_or_fundamental(tcx, trait_ref) { debug!("trait_ref_is_knowable: trait is neither local nor fundamental"); return false; } @@ -157,6 +154,11 @@ pub fn trait_ref_is_knowable<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err() } +pub fn trait_ref_is_local_or_fundamental<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>, + trait_ref: &ty::TraitRef<'tcx>) { + trait_ref.def_id.krate == LOCAL_CRATE || tcx.has_attr(trait_ref.def_id, "fundamental") +} + pub enum OrphanCheckErr<'tcx> { NoLocalInputType, UncoveredTy(Ty<'tcx>), diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index a5a95209be035..9268ea302cbd5 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -1077,9 +1077,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } else { None }; - let cause = if - trait_ref.def_id.krate != LOCAL_CRATE && - !self.tcx().has_attr(trait_ref.def_id, "fundamental") { + let cause = if !coherence::trait_ref_is_local_or_fundamental(self.tcx(), + trait_ref) { IntercrateAmbiguityCause::UpstreamCrateUpdate { trait_desc, self_desc } } else { IntercrateAmbiguityCause::DownstreamCrate { trait_desc, self_desc } From 4cd5314dd59c49a74cf1afebf9fac54ba01a8926 Mon Sep 17 00:00:00 2001 From: steveklabnik Date: Mon, 28 Aug 2017 19:30:45 -0400 Subject: [PATCH 16/17] Deprecate several flags in rustdoc Part of #44136 --- src/librustdoc/lib.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 61a8165d26af1..0816a9facc166 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -267,6 +267,9 @@ pub fn main_args(args: &[String]) -> isize { // Check for unstable options. nightly_options::check_nightly_options(&matches, &opts()); + // check for deprecated options + check_deprecated_options(&matches); + if matches.opt_present("h") || matches.opt_present("help") { usage("rustdoc"); return 0; @@ -538,3 +541,16 @@ where R: 'static + Send, F: 'static + Send + FnOnce(Output) -> R { }); rx.recv().unwrap() } + +/// Prints deprecation warnings for deprecated options +fn check_deprecated_options(matches: &getopts::Matches) { + if matches.opt_present("input-format") || + matches.opt_present("output-format") || + matches.opt_present("plugin-path") || + matches.opt_present("plugins") || + matches.opt_present("no-defaults") || + matches.opt_present("passes") { + eprintln!("WARNING: this flag is considered deprecated"); + eprintln!("WARNING: please see https://github.com/rust-lang/rust/issues/44136"); + } +} From ecd127d23fd035995749cf570313b3f1626ad13d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 28 Aug 2017 18:40:57 -0700 Subject: [PATCH 17/17] rustbuild: Fix dependencies of build-manifest No need to depend on librustc! All we need is libstd Closes #44140 --- src/bootstrap/tool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index d798e8de3dffa..e759f1a3e6f85 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -198,7 +198,7 @@ tool!( Linkchecker, "src/tools/linkchecker", "linkchecker", Mode::Libstd; CargoTest, "src/tools/cargotest", "cargotest", Mode::Libstd; Compiletest, "src/tools/compiletest", "compiletest", Mode::Libtest; - BuildManifest, "src/tools/build-manifest", "build-manifest", Mode::Librustc; + BuildManifest, "src/tools/build-manifest", "build-manifest", Mode::Libstd; RemoteTestClient, "src/tools/remote-test-client", "remote-test-client", Mode::Libstd; RustInstaller, "src/tools/rust-installer", "rust-installer", Mode::Libstd; );