diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 2aa3f9c7ec04b..9e56dd3770d46 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -79,6 +79,7 @@ def _download(path, url, probably_big, verbose, exception): option = "-#" else: option = "-s" + require(["curl", "--version"]) run(["curl", option, "-y", "30", "-Y", "10", # timeout if speed is < 10 bytes/sec for > 30 seconds "--connect-timeout", "30", # timeout if cannot connect within 30 seconds @@ -143,6 +144,21 @@ def run(args, verbose=False, exception=False, **kwargs): sys.exit(err) +def require(cmd, exit=True): + '''Run a command, returning its output. + On error, + If `exit` is `True`, exit the process. + Otherwise, return None.''' + try: + return subprocess.check_output(cmd).strip() + except (subprocess.CalledProcessError, OSError) as exc: + if not exit: + return None + print("error: unable to run `{}`: {}".format(' '.join(cmd), exc)) + print("Please make sure it's installed and in the path.") + sys.exit(1) + + def stage0_data(rust_root): """Build a dictionary from stage0.txt""" nightlies = os.path.join(rust_root, "src/stage0.txt") @@ -164,16 +180,12 @@ def format_build_time(duration): def default_build_triple(): """Build triple as in LLVM""" default_encoding = sys.getdefaultencoding() - try: - ostype = subprocess.check_output( - ['uname', '-s']).strip().decode(default_encoding) - cputype = subprocess.check_output( - ['uname', '-m']).strip().decode(default_encoding) - except (subprocess.CalledProcessError, OSError): - if sys.platform == 'win32': - return 'x86_64-pc-windows-msvc' - err = "uname not found" - sys.exit(err) + required = not sys.platform == 'win32' + ostype = require(["uname", "-s"], exit=required).decode(default_encoding) + cputype = require(['uname', '-m'], exit=required).decode(default_encoding) + + if ostype is None or cputype is None: + return 'x86_64-pc-windows-msvc' # The goal here is to come up with the same triple as LLVM would, # at least for the subset of platforms we're willing to target. @@ -203,12 +215,7 @@ def default_build_triple(): # output from that option is too generic for our purposes (it will # always emit 'i386' on x86/amd64 systems). As such, isainfo -k # must be used instead. - try: - cputype = subprocess.check_output( - ['isainfo', '-k']).strip().decode(default_encoding) - except (subprocess.CalledProcessError, OSError): - err = "isainfo not found" - sys.exit(err) + cputype = require(['isainfo', '-k']).decode(default_encoding) elif ostype.startswith('MINGW'): # msys' `uname` does not print gcc configuration, but prints msys # configuration. so we cannot believe `uname -m`: @@ -766,13 +773,8 @@ def update_submodules(self): default_encoding = sys.getdefaultencoding() # check the existence and version of 'git' command - try: - git_version_output = subprocess.check_output(['git', '--version']) - git_version_str = git_version_output.strip().split()[2].decode(default_encoding) - self.git_version = distutils.version.LooseVersion(git_version_str) - except (subprocess.CalledProcessError, OSError): - print("error: `git` is not found, please make sure it's installed and in the path.") - sys.exit(1) + git_version_str = require(['git', '--version']).split()[2].decode(default_encoding) + self.git_version = distutils.version.LooseVersion(git_version_str) slow_submodules = self.get_toml('fast-submodules') == "false" start_time = time() diff --git a/src/bootstrap/flags.rs b/src/bootstrap/flags.rs index 5d6e401d5b3fb..fb380af0a47e7 100644 --- a/src/bootstrap/flags.rs +++ b/src/bootstrap/flags.rs @@ -106,18 +106,18 @@ impl Flags { Usage: x.py [options] [...] Subcommands: - build Compile either the compiler or libraries - check Compile either the compiler or libraries, using cargo check + build, b Compile either the compiler or libraries + check, c Compile either the compiler or libraries, using cargo check clippy Run clippy (uses rustup/cargo-installed clippy binary) fix Run cargo fix fmt Run rustfmt - test Build and run some test suites + test, t Build and run some test suites bench Build and run some benchmarks doc Build documentation clean Clean out build directories dist Build distribution artifacts install Install distribution artifacts - run Run tools contained in this repository + run, r Run tools contained in this repository To learn more about a subcommand, run `./x.py -h`", ); @@ -184,17 +184,21 @@ To learn more about a subcommand, run `./x.py -h`", // there on out. let subcommand = args.iter().find(|&s| { (s == "build") + || (s == "b") || (s == "check") + || (s == "c") || (s == "clippy") || (s == "fix") || (s == "fmt") || (s == "test") + || (s == "t") || (s == "bench") || (s == "doc") || (s == "clean") || (s == "dist") || (s == "install") || (s == "run") + || (s == "r") }); let subcommand = match subcommand { Some(s) => s, @@ -210,7 +214,7 @@ To learn more about a subcommand, run `./x.py -h`", // Some subcommands get extra options match subcommand.as_str() { - "test" => { + "test" | "t" => { opts.optflag("", "no-fail-fast", "Run all tests regardless of failure"); opts.optmulti("", "test-args", "extra arguments", "ARGS"); opts.optmulti( @@ -285,7 +289,7 @@ To learn more about a subcommand, run `./x.py -h`", } // Extra help text for some commands match subcommand.as_str() { - "build" => { + "build" | "b" => { subcommand_help.push_str( "\n Arguments: @@ -312,7 +316,7 @@ Arguments: Once this is done, build/$ARCH/stage1 contains a usable compiler.", ); } - "check" => { + "check" | "c" => { subcommand_help.push_str( "\n Arguments: @@ -362,7 +366,7 @@ Arguments: ./x.py fmt --check", ); } - "test" => { + "test" | "t" => { subcommand_help.push_str( "\n Arguments: @@ -407,7 +411,7 @@ Arguments: ./x.py doc --stage 1", ); } - "run" => { + "run" | "r" => { subcommand_help.push_str( "\n Arguments: @@ -453,11 +457,11 @@ Arguments: } let cmd = match subcommand.as_str() { - "build" => Subcommand::Build { paths }, - "check" => Subcommand::Check { paths }, + "build" | "b" => Subcommand::Build { paths }, + "check" | "c" => Subcommand::Check { paths }, "clippy" => Subcommand::Clippy { paths }, "fix" => Subcommand::Fix { paths }, - "test" => Subcommand::Test { + "test" | "t" => Subcommand::Test { paths, bless: matches.opt_present("bless"), compare_mode: matches.opt_str("compare-mode"), @@ -487,7 +491,7 @@ Arguments: "fmt" => Subcommand::Format { check: matches.opt_present("check") }, "dist" => Subcommand::Dist { paths }, "install" => Subcommand::Install { paths }, - "run" => { + "run" | "r" => { if paths.is_empty() { println!("\nrun requires at least a path!\n"); usage(1, &opts, &subcommand_help, &extra_help); diff --git a/src/liballoc/benches/btree/map.rs b/src/liballoc/benches/btree/map.rs index 83cdebf0e3f4a..38d19c59ad186 100644 --- a/src/liballoc/benches/btree/map.rs +++ b/src/liballoc/benches/btree/map.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; use std::iter::Iterator; -use std::ops::Bound::{Excluded, Unbounded}; +use std::ops::RangeBounds; use std::vec::Vec; use rand::{seq::SliceRandom, thread_rng, Rng}; @@ -117,7 +117,7 @@ map_find_rand_bench! {find_rand_10_000, 10_000, BTreeMap} map_find_seq_bench! {find_seq_100, 100, BTreeMap} map_find_seq_bench! {find_seq_10_000, 10_000, BTreeMap} -fn bench_iter(b: &mut Bencher, size: i32) { +fn bench_iteration(b: &mut Bencher, size: i32) { let mut map = BTreeMap::::new(); let mut rng = thread_rng(); @@ -133,21 +133,21 @@ fn bench_iter(b: &mut Bencher, size: i32) { } #[bench] -pub fn iter_20(b: &mut Bencher) { - bench_iter(b, 20); +pub fn iteration_20(b: &mut Bencher) { + bench_iteration(b, 20); } #[bench] -pub fn iter_1000(b: &mut Bencher) { - bench_iter(b, 1000); +pub fn iteration_1000(b: &mut Bencher) { + bench_iteration(b, 1000); } #[bench] -pub fn iter_100000(b: &mut Bencher) { - bench_iter(b, 100000); +pub fn iteration_100000(b: &mut Bencher) { + bench_iteration(b, 100000); } -fn bench_iter_mut(b: &mut Bencher, size: i32) { +fn bench_iteration_mut(b: &mut Bencher, size: i32) { let mut map = BTreeMap::::new(); let mut rng = thread_rng(); @@ -163,18 +163,18 @@ fn bench_iter_mut(b: &mut Bencher, size: i32) { } #[bench] -pub fn iter_mut_20(b: &mut Bencher) { - bench_iter_mut(b, 20); +pub fn iteration_mut_20(b: &mut Bencher) { + bench_iteration_mut(b, 20); } #[bench] -pub fn iter_mut_1000(b: &mut Bencher) { - bench_iter_mut(b, 1000); +pub fn iteration_mut_1000(b: &mut Bencher) { + bench_iteration_mut(b, 1000); } #[bench] -pub fn iter_mut_100000(b: &mut Bencher) { - bench_iter_mut(b, 100000); +pub fn iteration_mut_100000(b: &mut Bencher) { + bench_iteration_mut(b, 100000); } fn bench_first_and_last(b: &mut Bencher, size: i32) { @@ -202,57 +202,83 @@ pub fn first_and_last_10k(b: &mut Bencher) { bench_first_and_last(b, 10_000); } -#[bench] -pub fn range_excluded_excluded(b: &mut Bencher) { - let size = 144; - let map: BTreeMap<_, _> = (0..size).map(|i| (i, i)).collect(); +const BENCH_RANGE_SIZE: i32 = 145; +const BENCH_RANGE_COUNT: i32 = BENCH_RANGE_SIZE * (BENCH_RANGE_SIZE - 1) / 2; + +fn bench_range(b: &mut Bencher, f: F) +where + F: Fn(i32, i32) -> R, + R: RangeBounds, +{ + let map: BTreeMap<_, _> = (0..BENCH_RANGE_SIZE).map(|i| (i, i)).collect(); b.iter(|| { - for first in 0..size { - for last in first + 1..size { - black_box(map.range((Excluded(first), Excluded(last)))); + let mut c = 0; + for i in 0..BENCH_RANGE_SIZE { + for j in i + 1..BENCH_RANGE_SIZE { + black_box(map.range(f(i, j))); + c += 1; } } + debug_assert_eq!(c, BENCH_RANGE_COUNT); }); } #[bench] -pub fn range_excluded_unbounded(b: &mut Bencher) { - let size = 144; - let map: BTreeMap<_, _> = (0..size).map(|i| (i, i)).collect(); - b.iter(|| { - for first in 0..size { - black_box(map.range((Excluded(first), Unbounded))); - } - }); +pub fn range_included_excluded(b: &mut Bencher) { + bench_range(b, |i, j| i..j); } #[bench] pub fn range_included_included(b: &mut Bencher) { - let size = 144; - let map: BTreeMap<_, _> = (0..size).map(|i| (i, i)).collect(); - b.iter(|| { - for first in 0..size { - for last in first..size { - black_box(map.range(first..=last)); - } - } - }); + bench_range(b, |i, j| i..=j); } #[bench] pub fn range_included_unbounded(b: &mut Bencher) { - let size = 144; + bench_range(b, |i, _| i..); +} + +#[bench] +pub fn range_unbounded_unbounded(b: &mut Bencher) { + bench_range(b, |_, _| ..); +} + +fn bench_iter(b: &mut Bencher, repeats: i32, size: i32) { let map: BTreeMap<_, _> = (0..size).map(|i| (i, i)).collect(); b.iter(|| { - for first in 0..size { - black_box(map.range(first..)); + for _ in 0..repeats { + black_box(map.iter()); } }); } +/// Contrast range_unbounded_unbounded with `iter()`. #[bench] -pub fn range_unbounded_unbounded(b: &mut Bencher) { - let size = 144; - let map: BTreeMap<_, _> = (0..size).map(|i| (i, i)).collect(); - b.iter(|| map.range(..)); +pub fn range_unbounded_vs_iter(b: &mut Bencher) { + bench_iter(b, BENCH_RANGE_COUNT, BENCH_RANGE_SIZE); +} + +#[bench] +pub fn iter_0(b: &mut Bencher) { + bench_iter(b, 1_000, 0); +} + +#[bench] +pub fn iter_1(b: &mut Bencher) { + bench_iter(b, 1_000, 1); +} + +#[bench] +pub fn iter_100(b: &mut Bencher) { + bench_iter(b, 1_000, 100); +} + +#[bench] +pub fn iter_10k(b: &mut Bencher) { + bench_iter(b, 1_000, 10_000); +} + +#[bench] +pub fn iter_1m(b: &mut Bencher) { + bench_iter(b, 1_000, 1_000_000); } diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index b8f1a4199c64f..98a94d695f784 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -1540,16 +1540,10 @@ impl IntoIterator for BTreeMap { fn into_iter(self) -> IntoIter { let mut me = ManuallyDrop::new(self); - if let Some(root) = me.root.as_mut() { - let root1 = unsafe { ptr::read(root).into_ref() }; - let root2 = unsafe { ptr::read(root).into_ref() }; - let len = me.length; - - IntoIter { - front: Some(root1.first_leaf_edge()), - back: Some(root2.last_leaf_edge()), - length: len, - } + if let Some(root) = me.root.take() { + let (f, b) = full_range_search(root.into_ref()); + + IntoIter { front: Some(f), back: Some(b), length: me.length } } else { IntoIter { front: None, back: None, length: 0 } } @@ -2037,6 +2031,7 @@ where } } +/// Finds the leaf edges delimiting a specified range in or underneath a node. fn range_search>( root: NodeRef, range: R, @@ -2122,6 +2117,33 @@ where } } +/// Equivalent to `range_search(k, v, ..)` without the `Ord` bound. +fn full_range_search( + root: NodeRef, +) -> ( + Handle, marker::Edge>, + Handle, marker::Edge>, +) { + // We duplicate the root NodeRef here -- we will never access it in a way + // that overlaps references obtained from the root. + let mut min_node = unsafe { ptr::read(&root) }; + let mut max_node = root; + loop { + let front = min_node.first_edge(); + let back = max_node.last_edge(); + match (front.force(), back.force()) { + (Leaf(f), Leaf(b)) => { + return (f, b); + } + (Internal(min_int), Internal(max_int)) => { + min_node = min_int.descend(); + max_node = max_int.descend(); + } + _ => unreachable!("BTreeMap has different depths"), + }; + } +} + impl BTreeMap { /// Gets an iterator over the entries of the map, sorted by key. /// @@ -2146,12 +2168,12 @@ impl BTreeMap { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn iter(&self) -> Iter<'_, K, V> { - Iter { - range: Range { - front: self.root.as_ref().map(|r| r.as_ref().first_leaf_edge()), - back: self.root.as_ref().map(|r| r.as_ref().last_leaf_edge()), - }, - length: self.length, + if let Some(root) = &self.root { + let (f, b) = full_range_search(root.as_ref()); + + Iter { range: Range { front: Some(f), back: Some(b) }, length: self.length } + } else { + Iter { range: Range { front: None, back: None }, length: 0 } } } @@ -2178,19 +2200,15 @@ impl BTreeMap { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn iter_mut(&mut self) -> IterMut<'_, K, V> { - IterMut { - range: if let Some(root) = &mut self.root { - let root1 = root.as_mut(); - let root2 = unsafe { ptr::read(&root1) }; - RangeMut { - front: Some(root1.first_leaf_edge()), - back: Some(root2.last_leaf_edge()), - _marker: PhantomData, - } - } else { - RangeMut { front: None, back: None, _marker: PhantomData } - }, - length: self.length, + if let Some(root) = &mut self.root { + let (f, b) = full_range_search(root.as_mut()); + + IterMut { + range: RangeMut { front: Some(f), back: Some(b), _marker: PhantomData }, + length: self.length, + } + } else { + IterMut { range: RangeMut { front: None, back: None, _marker: PhantomData }, length: 0 } } } diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 395fd7460850f..cc88fbb295c68 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -23,6 +23,7 @@ use rustc_session::Session; use rustc_span::symbol::{kw, sym}; use rustc_span::Span; use std::mem; +use std::ops::DerefMut; const MORE_EXTERN: &str = "for more information, visit https://doc.rust-lang.org/std/keyword.extern.html"; @@ -1113,17 +1114,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { for predicate in &generics.where_clause.predicates { if let WherePredicate::EqPredicate(ref predicate) = *predicate { - self.err_handler() - .struct_span_err( - predicate.span, - "equality constraints are not yet supported in `where` clauses", - ) - .span_label(predicate.span, "not supported") - .note( - "see issue #20041 \ - for more information", - ) - .emit(); + deny_equality_constraints(self, predicate, generics); } } @@ -1300,6 +1291,89 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } } +/// When encountering an equality constraint in a `where` clause, emit an error. If the code seems +/// like it's setting an associated type, provide an appropriate suggestion. +fn deny_equality_constraints( + this: &mut AstValidator<'_>, + predicate: &WhereEqPredicate, + generics: &Generics, +) { + let mut err = this.err_handler().struct_span_err( + predicate.span, + "equality constraints are not yet supported in `where` clauses", + ); + err.span_label(predicate.span, "not supported"); + + // Given `::Bar = RhsTy`, suggest `A: Foo`. + if let TyKind::Path(Some(qself), full_path) = &predicate.lhs_ty.kind { + if let TyKind::Path(None, path) = &qself.ty.kind { + match &path.segments[..] { + [PathSegment { ident, args: None, .. }] => { + for param in &generics.params { + if param.ident == *ident { + let param = ident; + match &full_path.segments[qself.position..] { + [PathSegment { ident, .. }] => { + // Make a new `Path` from `foo::Bar` to `Foo`. + let mut assoc_path = full_path.clone(); + // Remove `Bar` from `Foo::Bar`. + assoc_path.segments.pop(); + let len = assoc_path.segments.len() - 1; + // Build ``. + let arg = AngleBracketedArg::Constraint(AssocTyConstraint { + id: rustc_ast::node_id::DUMMY_NODE_ID, + ident: *ident, + kind: AssocTyConstraintKind::Equality { + ty: predicate.rhs_ty.clone(), + }, + span: ident.span, + }); + // Add `` to `Foo`. + match &mut assoc_path.segments[len].args { + Some(args) => match args.deref_mut() { + GenericArgs::Parenthesized(_) => continue, + GenericArgs::AngleBracketed(args) => { + args.args.push(arg); + } + }, + empty_args => { + *empty_args = AngleBracketedArgs { + span: ident.span, + args: vec![arg], + } + .into(); + } + } + err.span_suggestion_verbose( + predicate.span, + &format!( + "if `{}` is an associated type you're trying to set, \ + use the associated type binding syntax", + ident + ), + format!( + "{}: {}", + param, + pprust::path_to_string(&assoc_path) + ), + Applicability::MaybeIncorrect, + ); + } + _ => {} + }; + } + } + } + _ => {} + } + } + } + err.note( + "see issue #20041 for more information", + ); + err.emit(); +} + pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) -> bool { let mut validator = AstValidator { session, diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 258428d77da10..e6d673b30f7bc 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -2626,8 +2626,42 @@ impl Node<'_> { match self { Node::TraitItem(TraitItem { generics, .. }) | Node::ImplItem(ImplItem { generics, .. }) - | Node::Item(Item { kind: ItemKind::Fn(_, generics, _), .. }) => Some(generics), + | Node::Item(Item { + kind: + ItemKind::Trait(_, _, generics, ..) + | ItemKind::Impl { generics, .. } + | ItemKind::Fn(_, generics, _), + .. + }) => Some(generics), _ => None, } } + + pub fn hir_id(&self) -> Option { + match self { + Node::Item(Item { hir_id, .. }) + | Node::ForeignItem(ForeignItem { hir_id, .. }) + | Node::TraitItem(TraitItem { hir_id, .. }) + | Node::ImplItem(ImplItem { hir_id, .. }) + | Node::Field(StructField { hir_id, .. }) + | Node::AnonConst(AnonConst { hir_id, .. }) + | Node::Expr(Expr { hir_id, .. }) + | Node::Stmt(Stmt { hir_id, .. }) + | Node::Ty(Ty { hir_id, .. }) + | Node::Binding(Pat { hir_id, .. }) + | Node::Pat(Pat { hir_id, .. }) + | Node::Arm(Arm { hir_id, .. }) + | Node::Block(Block { hir_id, .. }) + | Node::Local(Local { hir_id, .. }) + | Node::MacroDef(MacroDef { hir_id, .. }) + | Node::Lifetime(Lifetime { hir_id, .. }) + | Node::Param(Param { hir_id, .. }) + | Node::GenericParam(GenericParam { hir_id, .. }) => Some(*hir_id), + Node::TraitRef(TraitRef { hir_ref_id, .. }) => Some(*hir_ref_id), + Node::PathSegment(PathSegment { hir_id, .. }) => *hir_id, + Node::Variant(Variant { id, .. }) => Some(*id), + Node::Ctor(variant) => variant.ctor_hir_id(), + Node::Crate(_) | Node::Visibility(_) => None, + } + } } diff --git a/src/librustc_middle/ty/diagnostics.rs b/src/librustc_middle/ty/diagnostics.rs index 790eb8f49aff3..613d66d59c55b 100644 --- a/src/librustc_middle/ty/diagnostics.rs +++ b/src/librustc_middle/ty/diagnostics.rs @@ -2,7 +2,12 @@ use crate::ty::sty::InferTy; use crate::ty::TyKind::*; -use crate::ty::TyS; +use crate::ty::{TyCtxt, TyS}; +use rustc_errors::{Applicability, DiagnosticBuilder}; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_hir::{QPath, TyKind, WhereBoundPredicate, WherePredicate}; +use rustc_span::{BytePos, Span}; impl<'tcx> TyS<'tcx> { /// Similar to `TyS::is_primitive`, but also considers inferred numeric values to be primitive. @@ -67,3 +72,180 @@ impl<'tcx> TyS<'tcx> { } } } + +/// Suggest restricting a type param with a new bound. +pub fn suggest_constraining_type_param( + tcx: TyCtxt<'_>, + generics: &hir::Generics<'_>, + err: &mut DiagnosticBuilder<'_>, + param_name: &str, + constraint: &str, + def_id: Option, +) -> bool { + let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name); + + let param = if let Some(param) = param { + param + } else { + return false; + }; + + const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound"; + let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name); + let msg_restrict_type_further = + format!("consider further restricting type parameter `{}`", param_name); + + if def_id == tcx.lang_items().sized_trait() { + // Type parameters are already `Sized` by default. + err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint)); + return true; + } + let mut suggest_restrict = |span| { + err.span_suggestion_verbose( + span, + MSG_RESTRICT_BOUND_FURTHER, + format!(" + {}", constraint), + Applicability::MachineApplicable, + ); + }; + + if param_name.starts_with("impl ") { + // If there's an `impl Trait` used in argument position, suggest + // restricting it: + // + // fn foo(t: impl Foo) { ... } + // -------- + // | + // help: consider further restricting this bound with `+ Bar` + // + // Suggestion for tools in this case is: + // + // fn foo(t: impl Foo) { ... } + // -------- + // | + // replace with: `impl Foo + Bar` + + suggest_restrict(param.span.shrink_to_hi()); + return true; + } + + if generics.where_clause.predicates.is_empty() + // Given `trait Base: Super` where `T: Copy`, suggest restricting in the + // `where` clause instead of `trait Base: Super`. + && !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) + { + if let Some(bounds_span) = param.bounds_span() { + // If user has provided some bounds, suggest restricting them: + // + // fn foo(t: T) { ... } + // --- + // | + // help: consider further restricting this bound with `+ Bar` + // + // Suggestion for tools in this case is: + // + // fn foo(t: T) { ... } + // -- + // | + // replace with: `T: Bar +` + suggest_restrict(bounds_span.shrink_to_hi()); + } else { + // If user hasn't provided any bounds, suggest adding a new one: + // + // fn foo(t: T) { ... } + // - help: consider restricting this type parameter with `T: Foo` + err.span_suggestion_verbose( + param.span.shrink_to_hi(), + &msg_restrict_type, + format!(": {}", constraint), + Applicability::MachineApplicable, + ); + } + + true + } else { + // This part is a bit tricky, because using the `where` clause user can + // provide zero, one or many bounds for the same type parameter, so we + // have following cases to consider: + // + // 1) When the type parameter has been provided zero bounds + // + // Message: + // fn foo(x: X, y: Y) where Y: Foo { ... } + // - help: consider restricting this type parameter with `where X: Bar` + // + // Suggestion: + // fn foo(x: X, y: Y) where Y: Foo { ... } + // - insert: `, X: Bar` + // + // + // 2) When the type parameter has been provided one bound + // + // Message: + // fn foo(t: T) where T: Foo { ... } + // ^^^^^^ + // | + // help: consider further restricting this bound with `+ Bar` + // + // Suggestion: + // fn foo(t: T) where T: Foo { ... } + // ^^ + // | + // replace with: `T: Bar +` + // + // + // 3) When the type parameter has been provided many bounds + // + // Message: + // fn foo(t: T) where T: Foo, T: Bar {... } + // - help: consider further restricting this type parameter with `where T: Zar` + // + // Suggestion: + // fn foo(t: T) where T: Foo, T: Bar {... } + // - insert: `, T: Zar` + + let mut param_spans = Vec::new(); + + for predicate in generics.where_clause.predicates { + if let WherePredicate::BoundPredicate(WhereBoundPredicate { + span, bounded_ty, .. + }) = predicate + { + if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind { + if let Some(segment) = path.segments.first() { + if segment.ident.to_string() == param_name { + param_spans.push(span); + } + } + } + } + } + + let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place(); + // Account for `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. + let mut trailing_comma = false; + if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) { + trailing_comma = snippet.ends_with(','); + } + let where_clause_span = if trailing_comma { + let hi = where_clause_span.hi(); + Span::new(hi - BytePos(1), hi, where_clause_span.ctxt()) + } else { + where_clause_span.shrink_to_hi() + }; + + match ¶m_spans[..] { + &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), + _ => { + err.span_suggestion_verbose( + where_clause_span, + &msg_restrict_type_further, + format!(", {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } + } + + true + } +} diff --git a/src/librustc_middle/ty/error.rs b/src/librustc_middle/ty/error.rs index 4e1a8b0e92f13..f3b6a53dfeb82 100644 --- a/src/librustc_middle/ty/error.rs +++ b/src/librustc_middle/ty/error.rs @@ -1,4 +1,5 @@ use crate::traits::{ObligationCause, ObligationCauseCode}; +use crate::ty::diagnostics::suggest_constraining_type_param; use crate::ty::{self, BoundRegion, Region, Ty, TyCtxt}; use rustc_ast::ast; use rustc_errors::Applicability::{MachineApplicable, MaybeIncorrect}; @@ -401,8 +402,46 @@ impl<'tcx> TyCtxt<'tcx> { (ty::Projection(_), ty::Projection(_)) => { db.note("an associated type was expected, but a different one was found"); } - (ty::Param(_), ty::Projection(_)) | (ty::Projection(_), ty::Param(_)) => { - db.note("you might be missing a type parameter or trait bound"); + (ty::Param(p), ty::Projection(proj)) | (ty::Projection(proj), ty::Param(p)) => { + let generics = self.generics_of(body_owner_def_id); + let p_span = self.def_span(generics.type_param(p, self).def_id); + if !sp.contains(p_span) { + db.span_label(p_span, "this type parameter"); + } + let hir = self.hir(); + let mut note = true; + if let Some(generics) = generics + .type_param(p, self) + .def_id + .as_local() + .map(|id| hir.as_local_hir_id(id)) + .and_then(|id| self.hir().find(self.hir().get_parent_node(id))) + .as_ref() + .and_then(|node| node.generics()) + { + // Synthesize the associated type restriction `Add`. + // FIXME: extract this logic for use in other diagnostics. + let trait_ref = proj.trait_ref(self); + let path = + self.def_path_str_with_substs(trait_ref.def_id, trait_ref.substs); + let item_name = self.item_name(proj.item_def_id); + let path = if path.ends_with('>') { + format!("{}, {} = {}>", &path[..path.len() - 1], item_name, p) + } else { + format!("{}<{} = {}>", path, item_name, p) + }; + note = !suggest_constraining_type_param( + self, + generics, + db, + &format!("{}", proj.self_ty()), + &path, + None, + ); + } + if note { + db.note("you might be missing a type parameter or trait bound"); + } } (ty::Param(p), ty::Dynamic(..) | ty::Opaque(..)) | (ty::Dynamic(..) | ty::Opaque(..), ty::Param(p)) => { diff --git a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs index 5bc9f6df889c7..14a094b9d5273 100644 --- a/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs @@ -10,10 +10,9 @@ use rustc_middle::mir::{ FakeReadCause, Local, LocalDecl, LocalInfo, LocalKind, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, suggest_constraining_type_param, Ty}; use rustc_span::source_map::DesugaringKind; use rustc_span::Span; -use rustc_trait_selection::traits::error_reporting::suggest_constraining_type_param; use crate::dataflow::drop_flag_effects; use crate::dataflow::indexes::{MoveOutIndex, MovePathIndex}; diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index fa2af24c94534..19ed6b50f92a6 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -15,17 +15,16 @@ use rustc_data_structures::fx::FxHashMap; use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, ErrorReported}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; -use rustc_hir::{Node, QPath, TyKind, WhereBoundPredicate, WherePredicate}; +use rustc_hir::Node; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::ty::error::ExpectedFound; -use rustc_middle::ty::fast_reject; use rustc_middle::ty::fold::TypeFolder; -use rustc_middle::ty::SubtypePredicate; use rustc_middle::ty::{ - self, AdtKind, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, + self, fast_reject, AdtKind, SubtypePredicate, ToPolyTraitRef, ToPredicate, Ty, TyCtxt, + TypeFoldable, WithConstness, }; use rustc_session::DiagnosticMessageId; -use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP}; +use rustc_span::{ExpnKind, Span, DUMMY_SP}; use std::fmt; use crate::traits::query::evaluate_obligation::InferCtxtExt as _; @@ -1700,180 +1699,3 @@ impl ArgKind { } } } - -/// Suggest restricting a type param with a new bound. -pub fn suggest_constraining_type_param( - tcx: TyCtxt<'_>, - generics: &hir::Generics<'_>, - err: &mut DiagnosticBuilder<'_>, - param_name: &str, - constraint: &str, - def_id: Option, -) -> bool { - let param = generics.params.iter().find(|p| p.name.ident().as_str() == param_name); - - let param = if let Some(param) = param { - param - } else { - return false; - }; - - const MSG_RESTRICT_BOUND_FURTHER: &str = "consider further restricting this bound"; - let msg_restrict_type = format!("consider restricting type parameter `{}`", param_name); - let msg_restrict_type_further = - format!("consider further restricting type parameter `{}`", param_name); - - if def_id == tcx.lang_items().sized_trait() { - // Type parameters are already `Sized` by default. - err.span_label(param.span, &format!("this type parameter needs to be `{}`", constraint)); - return true; - } - let mut suggest_restrict = |span| { - err.span_suggestion_verbose( - span, - MSG_RESTRICT_BOUND_FURTHER, - format!(" + {}", constraint), - Applicability::MachineApplicable, - ); - }; - - if param_name.starts_with("impl ") { - // If there's an `impl Trait` used in argument position, suggest - // restricting it: - // - // fn foo(t: impl Foo) { ... } - // -------- - // | - // help: consider further restricting this bound with `+ Bar` - // - // Suggestion for tools in this case is: - // - // fn foo(t: impl Foo) { ... } - // -------- - // | - // replace with: `impl Foo + Bar` - - suggest_restrict(param.span.shrink_to_hi()); - return true; - } - - if generics.where_clause.predicates.is_empty() - // Given `trait Base: Super` where `T: Copy`, suggest restricting in the - // `where` clause instead of `trait Base: Super`. - && !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) - { - if let Some(bounds_span) = param.bounds_span() { - // If user has provided some bounds, suggest restricting them: - // - // fn foo(t: T) { ... } - // --- - // | - // help: consider further restricting this bound with `+ Bar` - // - // Suggestion for tools in this case is: - // - // fn foo(t: T) { ... } - // -- - // | - // replace with: `T: Bar +` - suggest_restrict(bounds_span.shrink_to_hi()); - } else { - // If user hasn't provided any bounds, suggest adding a new one: - // - // fn foo(t: T) { ... } - // - help: consider restricting this type parameter with `T: Foo` - err.span_suggestion_verbose( - param.span.shrink_to_hi(), - &msg_restrict_type, - format!(": {}", constraint), - Applicability::MachineApplicable, - ); - } - - true - } else { - // This part is a bit tricky, because using the `where` clause user can - // provide zero, one or many bounds for the same type parameter, so we - // have following cases to consider: - // - // 1) When the type parameter has been provided zero bounds - // - // Message: - // fn foo(x: X, y: Y) where Y: Foo { ... } - // - help: consider restricting this type parameter with `where X: Bar` - // - // Suggestion: - // fn foo(x: X, y: Y) where Y: Foo { ... } - // - insert: `, X: Bar` - // - // - // 2) When the type parameter has been provided one bound - // - // Message: - // fn foo(t: T) where T: Foo { ... } - // ^^^^^^ - // | - // help: consider further restricting this bound with `+ Bar` - // - // Suggestion: - // fn foo(t: T) where T: Foo { ... } - // ^^ - // | - // replace with: `T: Bar +` - // - // - // 3) When the type parameter has been provided many bounds - // - // Message: - // fn foo(t: T) where T: Foo, T: Bar {... } - // - help: consider further restricting this type parameter with `where T: Zar` - // - // Suggestion: - // fn foo(t: T) where T: Foo, T: Bar {... } - // - insert: `, T: Zar` - - let mut param_spans = Vec::new(); - - for predicate in generics.where_clause.predicates { - if let WherePredicate::BoundPredicate(WhereBoundPredicate { - span, bounded_ty, .. - }) = predicate - { - if let TyKind::Path(QPath::Resolved(_, path)) = &bounded_ty.kind { - if let Some(segment) = path.segments.first() { - if segment.ident.to_string() == param_name { - param_spans.push(span); - } - } - } - } - } - - let where_clause_span = generics.where_clause.span_for_predicates_or_empty_place(); - // Account for `fn foo(t: T) where T: Foo,` so we don't suggest two trailing commas. - let mut trailing_comma = false; - if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(where_clause_span) { - trailing_comma = snippet.ends_with(','); - } - let where_clause_span = if trailing_comma { - let hi = where_clause_span.hi(); - Span::new(hi - BytePos(1), hi, where_clause_span.ctxt()) - } else { - where_clause_span.shrink_to_hi() - }; - - match ¶m_spans[..] { - &[¶m_span] => suggest_restrict(param_span.shrink_to_hi()), - _ => { - err.span_suggestion_verbose( - where_clause_span, - &msg_restrict_type_further, - format!(", {}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } - } - - true - } -} diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index ce7b1390d46b6..74dd47a91c279 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -3,7 +3,6 @@ use super::{ }; use crate::infer::InferCtxt; -use crate::traits::error_reporting::suggest_constraining_type_param; use rustc_errors::{error_code, struct_span_err, Applicability, DiagnosticBuilder, Style}; use rustc_hir as hir; @@ -13,7 +12,8 @@ use rustc_hir::intravisit::Visitor; use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node}; use rustc_middle::ty::TypeckTables; use rustc_middle::ty::{ - self, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, + self, suggest_constraining_type_param, AdtKind, DefIdTree, Infer, InferTy, ToPredicate, Ty, + TyCtxt, TypeFoldable, WithConstness, }; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::{MultiSpan, Span, DUMMY_SP}; diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 23004cf364725..e6cbc8ab7230c 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -10,7 +10,7 @@ use rustc_middle::ty::adjustment::{ Adjust, Adjustment, AllowTwoPhase, AutoBorrow, AutoBorrowMutability, }; use rustc_middle::ty::TyKind::{Adt, Array, Char, FnDef, Never, Ref, Str, Tuple, Uint}; -use rustc_middle::ty::{self, Ty, TypeFoldable}; +use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt, TypeFoldable}; use rustc_span::Span; use rustc_trait_selection::infer::InferCtxtExt; @@ -253,6 +253,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // error types are considered "builtin" if !lhs_ty.references_error() && !rhs_ty.references_error() { let source_map = self.tcx.sess.source_map(); + match is_assign { IsAssign::Yes => { let mut err = struct_span_err!( @@ -317,12 +318,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " += "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(_) = lhs_ty.kind { - // FIXME: point to span of param - err.note(&format!( - "`{}` might need a bound for `{}`", - lhs_ty, missing_trait - )); + } else if let ty::Param(p) = lhs_ty.kind { + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + lhs_ty, + rhs_ty, + missing_trait, + p, + false, + ); } else if !suggested_deref { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } @@ -330,46 +336,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.emit(); } IsAssign::No => { - let (message, missing_trait) = match op.node { + let (message, missing_trait, use_output) = match op.node { hir::BinOpKind::Add => ( format!("cannot add `{}` to `{}`", rhs_ty, lhs_ty), Some("std::ops::Add"), + true, ), hir::BinOpKind::Sub => ( format!("cannot subtract `{}` from `{}`", rhs_ty, lhs_ty), Some("std::ops::Sub"), + true, ), hir::BinOpKind::Mul => ( format!("cannot multiply `{}` to `{}`", rhs_ty, lhs_ty), Some("std::ops::Mul"), + true, ), hir::BinOpKind::Div => ( format!("cannot divide `{}` by `{}`", lhs_ty, rhs_ty), Some("std::ops::Div"), + true, ), hir::BinOpKind::Rem => ( format!("cannot mod `{}` by `{}`", lhs_ty, rhs_ty), Some("std::ops::Rem"), + true, ), hir::BinOpKind::BitAnd => ( format!("no implementation for `{} & {}`", lhs_ty, rhs_ty), Some("std::ops::BitAnd"), + true, ), hir::BinOpKind::BitXor => ( format!("no implementation for `{} ^ {}`", lhs_ty, rhs_ty), Some("std::ops::BitXor"), + true, ), hir::BinOpKind::BitOr => ( format!("no implementation for `{} | {}`", lhs_ty, rhs_ty), Some("std::ops::BitOr"), + true, ), hir::BinOpKind::Shl => ( format!("no implementation for `{} << {}`", lhs_ty, rhs_ty), Some("std::ops::Shl"), + true, ), hir::BinOpKind::Shr => ( format!("no implementation for `{} >> {}`", lhs_ty, rhs_ty), Some("std::ops::Shr"), + true, ), hir::BinOpKind::Eq | hir::BinOpKind::Ne => ( format!( @@ -378,6 +394,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), Some("std::cmp::PartialEq"), + false, ), hir::BinOpKind::Lt | hir::BinOpKind::Le @@ -389,6 +406,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), Some("std::cmp::PartialOrd"), + false, ), _ => ( format!( @@ -397,6 +415,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { lhs_ty ), None, + false, ), }; let mut err = struct_span_err!( @@ -459,12 +478,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // This has nothing here because it means we did string // concatenation (e.g., "Hello " + "World!"). This means // we don't want the note in the else clause to be emitted - } else if let ty::Param(_) = lhs_ty.kind { - // FIXME: point to span of param - err.note(&format!( - "`{}` might need a bound for `{}`", - lhs_ty, missing_trait - )); + } else if let ty::Param(p) = lhs_ty.kind { + suggest_constraining_param( + self.tcx, + self.body_id, + &mut err, + lhs_ty, + rhs_ty, + missing_trait, + p, + use_output, + ); } else if !suggested_deref && !involves_fn { suggest_impl_missing(&mut err, lhs_ty, &missing_trait); } @@ -911,3 +935,43 @@ fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_tra } } } + +fn suggest_constraining_param( + tcx: TyCtxt<'_>, + body_id: hir::HirId, + mut err: &mut DiagnosticBuilder<'_>, + lhs_ty: Ty<'_>, + rhs_ty: Ty<'_>, + missing_trait: &str, + p: ty::ParamTy, + set_output: bool, +) { + let hir = tcx.hir(); + let msg = &format!("`{}` might need a bound for `{}`", lhs_ty, missing_trait); + // Try to find the def-id and details for the parameter p. We have only the index, + // so we have to find the enclosing function's def-id, then look through its declared + // generic parameters to get the declaration. + let def_id = hir.body_owner_def_id(hir::BodyId { hir_id: body_id }); + let generics = tcx.generics_of(def_id); + let param_def_id = generics.type_param(&p, tcx).def_id; + if let Some(generics) = param_def_id + .as_local() + .map(|id| hir.as_local_hir_id(id)) + .and_then(|id| hir.find(hir.get_parent_item(id))) + .as_ref() + .and_then(|node| node.generics()) + { + let output = if set_output { format!("", rhs_ty) } else { String::new() }; + suggest_constraining_type_param( + tcx, + generics, + &mut err, + &format!("{}", lhs_ty), + &format!("{}{}", missing_trait, output), + None, + ); + } else { + let span = tcx.def_span(param_def_id); + err.span_label(span, msg); + } +} diff --git a/src/libstd/sys/unix/mutex.rs b/src/libstd/sys/unix/mutex.rs index 103d87e3d2f91..45c600f75f5cf 100644 --- a/src/libstd/sys/unix/mutex.rs +++ b/src/libstd/sys/unix/mutex.rs @@ -28,14 +28,20 @@ impl Mutex { // // A pthread mutex initialized with PTHREAD_MUTEX_INITIALIZER will have // a type of PTHREAD_MUTEX_DEFAULT, which has undefined behavior if you - // try to re-lock it from the same thread when you already hold a lock. + // try to re-lock it from the same thread when you already hold a lock + // (https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html). + // This is the case even if PTHREAD_MUTEX_DEFAULT == PTHREAD_MUTEX_NORMAL + // (https://github.com/rust-lang/rust/issues/33770#issuecomment-220847521) -- in that + // case, `pthread_mutexattr_settype(PTHREAD_MUTEX_DEFAULT)` will of course be the same + // as setting it to `PTHREAD_MUTEX_NORMAL`, but not setting any mode will result in + // a Mutex where re-locking is UB. // // In practice, glibc takes advantage of this undefined behavior to // implement hardware lock elision, which uses hardware transactional // memory to avoid acquiring the lock. While a transaction is in // progress, the lock appears to be unlocked. This isn't a problem for // other threads since the transactional memory will abort if a conflict - // is detected, however no abort is generated if re-locking from the + // is detected, however no abort is generated when re-locking from the // same thread. // // Since locking the same mutex twice will result in two aliasing &mut diff --git a/src/libstd/sys/unix/rwlock.rs b/src/libstd/sys/unix/rwlock.rs index 079dea671ef76..2b5067a34f648 100644 --- a/src/libstd/sys/unix/rwlock.rs +++ b/src/libstd/sys/unix/rwlock.rs @@ -22,32 +22,33 @@ impl RWLock { pub unsafe fn read(&self) { let r = libc::pthread_rwlock_rdlock(self.inner.get()); - // According to the pthread_rwlock_rdlock spec, this function **may** - // fail with EDEADLK if a deadlock is detected. On the other hand - // pthread mutexes will *never* return EDEADLK if they are initialized - // as the "fast" kind (which ours always are). As a result, a deadlock - // situation may actually return from the call to pthread_rwlock_rdlock - // instead of blocking forever (as mutexes and Windows rwlocks do). Note - // that not all unix implementations, however, will return EDEADLK for - // their rwlocks. + // According to POSIX, when a thread tries to acquire this read lock + // while it already holds the write lock + // (or vice versa, or tries to acquire the write lock twice), + // "the call shall either deadlock or return [EDEADLK]" + // (https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_wrlock.html, + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_rdlock.html). + // So, in principle, all we have to do here is check `r == 0` to be sure we properly + // got the lock. // - // We roughly maintain the deadlocking behavior by panicking to ensure - // that this lock acquisition does not succeed. - // - // We also check whether this lock is already write locked. This - // is only possible if it was write locked by the current thread and - // the implementation allows recursive locking. The POSIX standard - // doesn't require recursively locking a rwlock to deadlock, but we can't - // allow that because it could lead to aliasing issues. + // However, (at least) glibc before version 2.25 does not conform to this spec, + // and can return `r == 0` even when this thread already holds the write lock. + // We thus check for this situation ourselves and panic when detecting that a thread + // got the write lock more than once, or got a read and a write lock. if r == libc::EAGAIN { panic!("rwlock maximum reader count exceeded"); } else if r == libc::EDEADLK || (r == 0 && *self.write_locked.get()) { + // Above, we make sure to only access `write_locked` when `r == 0` to avoid + // data races. if r == 0 { + // `pthread_rwlock_rdlock` succeeded when it should not have. self.raw_unlock(); } panic!("rwlock read lock would result in deadlock"); } else { - assert_eq!(r, 0); + // According to POSIX, for a properly initialized rwlock this can only + // return EAGAIN or EDEADLK or 0. We rely on that. + debug_assert_eq!(r, 0); self.num_readers.fetch_add(1, Ordering::Relaxed); } } @@ -56,6 +57,7 @@ impl RWLock { let r = libc::pthread_rwlock_tryrdlock(self.inner.get()); if r == 0 { if *self.write_locked.get() { + // `pthread_rwlock_tryrdlock` succeeded when it should not have. self.raw_unlock(); false } else { @@ -69,17 +71,22 @@ impl RWLock { #[inline] pub unsafe fn write(&self) { let r = libc::pthread_rwlock_wrlock(self.inner.get()); - // See comments above for why we check for EDEADLK and write_locked. We - // also need to check that num_readers is 0. + // See comments above for why we check for EDEADLK and write_locked. For the same reason, + // we also need to check that there are no readers (tracked in `num_readers`). if r == libc::EDEADLK - || *self.write_locked.get() + || (r == 0 && *self.write_locked.get()) || self.num_readers.load(Ordering::Relaxed) != 0 { + // Above, we make sure to only access `write_locked` when `r == 0` to avoid + // data races. if r == 0 { + // `pthread_rwlock_wrlock` succeeded when it should not have. self.raw_unlock(); } panic!("rwlock write lock would result in deadlock"); } else { + // According to POSIX, for a properly initialized rwlock this can only + // return EDEADLK or 0. We rely on that. debug_assert_eq!(r, 0); } *self.write_locked.get() = true; @@ -89,6 +96,7 @@ impl RWLock { let r = libc::pthread_rwlock_trywrlock(self.inner.get()); if r == 0 { if *self.write_locked.get() || self.num_readers.load(Ordering::Relaxed) != 0 { + // `pthread_rwlock_trywrlock` succeeded when it should not have. self.raw_unlock(); false } else { diff --git a/src/test/ui/deriving/deriving-hash.rs b/src/test/ui/deriving/deriving-hash.rs index 68c68c235ef4a..8b51370bca502 100644 --- a/src/test/ui/deriving/deriving-hash.rs +++ b/src/test/ui/deriving/deriving-hash.rs @@ -24,7 +24,7 @@ struct Person { enum E { A=1, B } fn hash(t: &T) -> u64 { - let mut s = SipHasher::new_with_keys(0, 0); + let mut s = SipHasher::new(); t.hash(&mut s); s.finish() } diff --git a/src/test/ui/generic-associated-types/construct_with_other_type.stderr b/src/test/ui/generic-associated-types/construct_with_other_type.stderr index bad746f7ef121..b9468b3330b44 100644 --- a/src/test/ui/generic-associated-types/construct_with_other_type.stderr +++ b/src/test/ui/generic-associated-types/construct_with_other_type.stderr @@ -2,11 +2,16 @@ error[E0271]: type mismatch resolving `for<'a> <::Baa<'a> as std::ops: --> $DIR/construct_with_other_type.rs:19:9 | LL | impl Baz for T where T: Foo { - | ^^^ expected type parameter `T`, found associated type + | - ^^^ expected type parameter `T`, found associated type + | | + | this type parameter | = note: expected associated type `::Bar<'_, 'static>` found associated type `<::Quux<'_> as Foo>::Bar<'_, 'static>` - = note: you might be missing a type parameter or trait bound +help: consider further restricting this bound + | +LL | impl Baz for T where T: Foo + Baz { + | ^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/generic-associated-types/missing-bounds.fixed b/src/test/ui/generic-associated-types/missing-bounds.fixed new file mode 100644 index 0000000000000..364d2388741b0 --- /dev/null +++ b/src/test/ui/generic-associated-types/missing-bounds.fixed @@ -0,0 +1,46 @@ +// run-rustfix + +use std::ops::Add; + +struct A(B); + +impl Add for A where B: Add + std::ops::Add { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + A(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +struct C(B); + +impl> Add for C { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +struct D(B); + +impl> Add for D { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR cannot add `B` to `B` + } +} + +struct E(B); + +impl Add for E where B: Add, B: std::ops::Add { + //~^ ERROR equality constraints are not yet supported in `where` clauses + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +fn main() {} diff --git a/src/test/ui/generic-associated-types/missing-bounds.rs b/src/test/ui/generic-associated-types/missing-bounds.rs new file mode 100644 index 0000000000000..ffafff5e9f586 --- /dev/null +++ b/src/test/ui/generic-associated-types/missing-bounds.rs @@ -0,0 +1,46 @@ +// run-rustfix + +use std::ops::Add; + +struct A(B); + +impl Add for A where B: Add { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + A(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +struct C(B); + +impl Add for C { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +struct D(B); + +impl Add for D { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR cannot add `B` to `B` + } +} + +struct E(B); + +impl Add for E where ::Output = B { + //~^ ERROR equality constraints are not yet supported in `where` clauses + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) //~ ERROR mismatched types + } +} + +fn main() {} diff --git a/src/test/ui/generic-associated-types/missing-bounds.stderr b/src/test/ui/generic-associated-types/missing-bounds.stderr new file mode 100644 index 0000000000000..50536fdaca96e --- /dev/null +++ b/src/test/ui/generic-associated-types/missing-bounds.stderr @@ -0,0 +1,77 @@ +error: equality constraints are not yet supported in `where` clauses + --> $DIR/missing-bounds.rs:37:33 + | +LL | impl Add for E where ::Output = B { + | ^^^^^^^^^^^^^^^^^^^^^^ not supported + | + = note: see issue #20041 for more information +help: if `Output` is an associated type you're trying to set, use the associated type binding syntax + | +LL | impl Add for E where B: Add { + | ^^^^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/missing-bounds.rs:11:11 + | +LL | impl Add for A where B: Add { + | - this type parameter +... +LL | A(self.0 + rhs.0) + | ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type + | + = note: expected type parameter `B` + found associated type `::Output` +help: consider further restricting this bound + | +LL | impl Add for A where B: Add + std::ops::Add { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/missing-bounds.rs:21:14 + | +LL | impl Add for C { + | - this type parameter +... +LL | Self(self.0 + rhs.0) + | ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type + | + = note: expected type parameter `B` + found associated type `::Output` +help: consider further restricting this bound + | +LL | impl> Add for C { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0369]: cannot add `B` to `B` + --> $DIR/missing-bounds.rs:31:21 + | +LL | Self(self.0 + rhs.0) + | ------ ^ ----- B + | | + | B + | +help: consider restricting type parameter `B` + | +LL | impl> Add for D { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0308]: mismatched types + --> $DIR/missing-bounds.rs:42:14 + | +LL | impl Add for E where ::Output = B { + | - this type parameter +... +LL | Self(self.0 + rhs.0) + | ^^^^^^^^^^^^^^ expected type parameter `B`, found associated type + | + = note: expected type parameter `B` + found associated type `::Output` +help: consider further restricting type parameter `B` + | +LL | impl Add for E where ::Output = B, B: std::ops::Add { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + +Some errors have detailed explanations: E0308, E0369. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/issues/issue-16530.rs b/src/test/ui/issues/issue-16530.rs index 22a6ef7fa091f..25817a2a63d60 100644 --- a/src/test/ui/issues/issue-16530.rs +++ b/src/test/ui/issues/issue-16530.rs @@ -7,9 +7,9 @@ use std::hash::{SipHasher, Hasher, Hash}; struct Empty; pub fn main() { - let mut s1 = SipHasher::new_with_keys(0, 0); + let mut s1 = SipHasher::new(); Empty.hash(&mut s1); - let mut s2 = SipHasher::new_with_keys(0, 0); + let mut s2 = SipHasher::new(); Empty.hash(&mut s2); assert_eq!(s1.finish(), s2.finish()); } diff --git a/src/test/ui/issues/issue-20005.stderr b/src/test/ui/issues/issue-20005.stderr index 19ccf7076199a..f53489a99f3b4 100644 --- a/src/test/ui/issues/issue-20005.stderr +++ b/src/test/ui/issues/issue-20005.stderr @@ -5,12 +5,18 @@ LL | trait From { | --- required by this bound in `From` ... LL | ) -> >::Result where Dst: From { - | ^^^^^^^^^^- help: consider further restricting `Self`: `, Self: std::marker::Sized` - | | - | doesn't have a size known at compile-time + | ^^^^^^^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `Self` = note: to learn more, visit +help: consider further restricting `Self` + | +LL | ) -> >::Result where Dst: From, Self: std::marker::Sized { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: consider relaxing the implicit `Sized` restriction + | +LL | trait From { + | ^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-24204.stderr b/src/test/ui/issues/issue-24204.stderr index d69efc860059b..d5cbcf786bf1a 100644 --- a/src/test/ui/issues/issue-24204.stderr +++ b/src/test/ui/issues/issue-24204.stderr @@ -7,7 +7,9 @@ LL | type A: MultiDispatch; | -------- required by this bound in `Trait` ... LL | fn test>(b: i32) -> T where T::A: MultiDispatch { T::new(b) } - | ^^^^^^^^^^^^ expected type parameter `T`, found associated type + | - ^^^^^^^^^^^^ expected type parameter `T`, found associated type + | | + | this type parameter | = note: expected type parameter `T` found associated type `<::A as MultiDispatch>::O` diff --git a/src/test/ui/issues/issue-6738.stderr b/src/test/ui/issues/issue-6738.stderr index 82b670bd03bc5..a428ff7e91fad 100644 --- a/src/test/ui/issues/issue-6738.stderr +++ b/src/test/ui/issues/issue-6738.stderr @@ -6,7 +6,10 @@ LL | self.x += v.x; | | | cannot use `+=` on type `T` | - = note: `T` might need a bound for `std::ops::AddAssign` +help: consider restricting type parameter `T` + | +LL | impl Foo { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/type/type-check/missing_trait_impl.stderr b/src/test/ui/type/type-check/missing_trait_impl.stderr index 7186d6a542dc9..30df1261cefa1 100644 --- a/src/test/ui/type/type-check/missing_trait_impl.stderr +++ b/src/test/ui/type/type-check/missing_trait_impl.stderr @@ -6,7 +6,10 @@ LL | let z = x + y; | | | T | - = note: `T` might need a bound for `std::ops::Add` +help: consider restricting type parameter `T` + | +LL | fn foo>(x: T, y: T) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0368]: binary assignment operation `+=` cannot be applied to type `T` --> $DIR/missing_trait_impl.rs:9:5 @@ -16,7 +19,10 @@ LL | x += x; | | | cannot use `+=` on type `T` | - = note: `T` might need a bound for `std::ops::AddAssign` +help: consider restricting type parameter `T` + | +LL | fn bar(x: T) { + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors