From 3b92689f3d0c3b90fa01d9873cdf01543d51c000 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 26 Dec 2019 13:54:33 -0500 Subject: [PATCH 01/18] Relax bounds on HashMap to match hashbrown No functional changes are made, and all APIs are moved to strictly less restrictive bounds. These APIs changed from the old bound listed to no trait bounds: K: Hash + Eq * new * with_capacity K: Eq + Hash, S: BuildHasher * with_hasher * with_capacity_and_hasher * hasher K: Eq + Hash + Debug -> K: Debug S: BuildHasher -> S K: Eq + Hash -> K S: BuildHasher + Default -> S: Default --- src/libstd/collections/hash/map.rs | 126 ++++++++++++++--------------- 1 file changed, 62 insertions(+), 64 deletions(-) diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index a928867d9de93..84e1cee5d66b9 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -203,7 +203,7 @@ pub struct HashMap { base: base::HashMap, } -impl HashMap { +impl HashMap { /// Creates an empty `HashMap`. /// /// The hash map is initially created with a capacity of 0, so it will not allocate until it @@ -240,6 +240,59 @@ impl HashMap { } impl HashMap { + /// Creates an empty `HashMap` which will use the given hash builder to hash + /// keys. + /// + /// The created map has the default initial capacity. + /// + /// Warning: `hash_builder` is normally randomly generated, and + /// is designed to allow HashMaps to be resistant to attacks that + /// cause many collisions and very poor performance. Setting it + /// manually using this function can expose a DoS attack vector. + /// + /// # Examples + /// + /// ``` + /// use std::collections::HashMap; + /// use std::collections::hash_map::RandomState; + /// + /// let s = RandomState::new(); + /// let mut map = HashMap::with_hasher(s); + /// map.insert(1, 2); + /// ``` + #[inline] + #[stable(feature = "hashmap_build_hasher", since = "1.7.0")] + pub fn with_hasher(hash_builder: S) -> HashMap { + HashMap { base: base::HashMap::with_hasher(hash_builder) } + } + + /// Creates an empty `HashMap` with the specified capacity, using `hash_builder` + /// to hash the keys. + /// + /// The hash map will be able to hold at least `capacity` elements without + /// reallocating. If `capacity` is 0, the hash map will not allocate. + /// + /// Warning: `hash_builder` is normally randomly generated, and + /// is designed to allow HashMaps to be resistant to attacks that + /// cause many collisions and very poor performance. Setting it + /// manually using this function can expose a DoS attack vector. + /// + /// # Examples + /// + /// ``` + /// use std::collections::HashMap; + /// use std::collections::hash_map::RandomState; + /// + /// let s = RandomState::new(); + /// let mut map = HashMap::with_capacity_and_hasher(10, s); + /// map.insert(1, 2); + /// ``` + #[inline] + #[stable(feature = "hashmap_build_hasher", since = "1.7.0")] + pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> HashMap { + HashMap { base: base::HashMap::with_capacity_and_hasher(capacity, hash_builder) } + } + /// Returns the number of elements the map can hold without reallocating. /// /// This number is a lower bound; the `HashMap` might be able to hold @@ -457,65 +510,6 @@ impl HashMap { pub fn clear(&mut self) { self.base.clear(); } -} - -impl HashMap -where - K: Eq + Hash, - S: BuildHasher, -{ - /// Creates an empty `HashMap` which will use the given hash builder to hash - /// keys. - /// - /// The created map has the default initial capacity. - /// - /// Warning: `hash_builder` is normally randomly generated, and - /// is designed to allow HashMaps to be resistant to attacks that - /// cause many collisions and very poor performance. Setting it - /// manually using this function can expose a DoS attack vector. - /// - /// # Examples - /// - /// ``` - /// use std::collections::HashMap; - /// use std::collections::hash_map::RandomState; - /// - /// let s = RandomState::new(); - /// let mut map = HashMap::with_hasher(s); - /// map.insert(1, 2); - /// ``` - #[inline] - #[stable(feature = "hashmap_build_hasher", since = "1.7.0")] - pub fn with_hasher(hash_builder: S) -> HashMap { - HashMap { base: base::HashMap::with_hasher(hash_builder) } - } - - /// Creates an empty `HashMap` with the specified capacity, using `hash_builder` - /// to hash the keys. - /// - /// The hash map will be able to hold at least `capacity` elements without - /// reallocating. If `capacity` is 0, the hash map will not allocate. - /// - /// Warning: `hash_builder` is normally randomly generated, and - /// is designed to allow HashMaps to be resistant to attacks that - /// cause many collisions and very poor performance. Setting it - /// manually using this function can expose a DoS attack vector. - /// - /// # Examples - /// - /// ``` - /// use std::collections::HashMap; - /// use std::collections::hash_map::RandomState; - /// - /// let s = RandomState::new(); - /// let mut map = HashMap::with_capacity_and_hasher(10, s); - /// map.insert(1, 2); - /// ``` - #[inline] - #[stable(feature = "hashmap_build_hasher", since = "1.7.0")] - pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> HashMap { - HashMap { base: base::HashMap::with_capacity_and_hasher(capacity, hash_builder) } - } /// Returns a reference to the map's [`BuildHasher`]. /// @@ -536,7 +530,13 @@ where pub fn hasher(&self) -> &S { self.base.hasher() } +} +impl HashMap +where + K: Eq + Hash, + S: BuildHasher, +{ /// Reserves capacity for at least `additional` more elements to be inserted /// in the `HashMap`. The collection may reserve more space to avoid /// frequent reallocations. @@ -984,9 +984,8 @@ where #[stable(feature = "rust1", since = "1.0.0")] impl Debug for HashMap where - K: Eq + Hash + Debug, + K: Debug, V: Debug, - S: BuildHasher, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_map().entries(self.iter()).finish() @@ -996,8 +995,7 @@ where #[stable(feature = "rust1", since = "1.0.0")] impl Default for HashMap where - K: Eq + Hash, - S: BuildHasher + Default, + S: Default, { /// Creates an empty `HashMap`, with the `Default` value for the hasher. #[inline] From 48859db151b839518bdd9d44a2387c0f6b65d141 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 26 Dec 2019 14:12:56 -0500 Subject: [PATCH 02/18] Relax bounds on HashSet to match hashbrown No functional changes are made, and all APIs are moved to strictly less restrictive bounds. These APIs changed from the old bound listed to the new bound: T: Hash + Eq -> T * new * with_capacity T: Eq + Hash, S: BuildHasher -> T * with_hasher * with_capacity_and_hasher * hasher T: Eq + Hash + Debug -> T: Debug S: BuildHasher -> S T: Eq + Hash -> T S: BuildHasher + Default -> S: Default --- src/libstd/collections/hash/set.rs | 20 +++++++++---------- .../core-traits-no-impls-length-33.rs | 1 - .../core-traits-no-impls-length-33.stderr | 19 +++++------------- 3 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/libstd/collections/hash/set.rs b/src/libstd/collections/hash/set.rs index fff64e9fc9008..f461f26572f01 100644 --- a/src/libstd/collections/hash/set.rs +++ b/src/libstd/collections/hash/set.rs @@ -110,7 +110,7 @@ pub struct HashSet { map: HashMap, } -impl HashSet { +impl HashSet { /// Creates an empty `HashSet`. /// /// The hash set is initially created with a capacity of 0, so it will not allocate until it @@ -261,13 +261,7 @@ impl HashSet { pub fn clear(&mut self) { self.map.clear() } -} -impl HashSet -where - T: Eq + Hash, - S: BuildHasher, -{ /// Creates a new empty hash set which will use the given hasher to hash /// keys. /// @@ -340,7 +334,13 @@ where pub fn hasher(&self) -> &S { self.map.hasher() } +} +impl HashSet +where + T: Eq + Hash, + S: BuildHasher, +{ /// Reserves capacity for at least `additional` more elements to be inserted /// in the `HashSet`. The collection may reserve more space to avoid /// frequent reallocations. @@ -896,8 +896,7 @@ where #[stable(feature = "rust1", since = "1.0.0")] impl fmt::Debug for HashSet where - T: Eq + Hash + fmt::Debug, - S: BuildHasher, + T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_set().entries(self.iter()).finish() @@ -945,8 +944,7 @@ where #[stable(feature = "rust1", since = "1.0.0")] impl Default for HashSet where - T: Eq + Hash, - S: BuildHasher + Default, + S: Default, { /// Creates an empty `HashSet` with the `Default` value for the hasher. #[inline] diff --git a/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.rs b/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.rs index 7fa059583f539..8397d204f35cf 100644 --- a/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.rs +++ b/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.rs @@ -6,7 +6,6 @@ pub fn no_debug() { pub fn no_hash() { use std::collections::HashSet; let mut set = HashSet::new(); - //~^ ERROR arrays only have std trait implementations for lengths 0..=32 set.insert([0_usize; 33]); //~^ ERROR arrays only have std trait implementations for lengths 0..=32 } diff --git a/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.stderr b/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.stderr index d885c98dcb287..594a0d4b5d844 100644 --- a/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.stderr +++ b/src/test/ui/const-generics/array-impls/core-traits-no-impls-length-33.stderr @@ -8,24 +8,15 @@ LL | println!("{:?}", [0_usize; 33]); = note: required by `std::fmt::Debug::fmt` error[E0277]: arrays only have std trait implementations for lengths 0..=32 - --> $DIR/core-traits-no-impls-length-33.rs:10:16 + --> $DIR/core-traits-no-impls-length-33.rs:9:16 | LL | set.insert([0_usize; 33]); | ^^^^^^^^^^^^^ the trait `std::array::LengthAtMost32` is not implemented for `[usize; 33]` | = note: required because of the requirements on the impl of `std::cmp::Eq` for `[usize; 33]` -error[E0277]: arrays only have std trait implementations for lengths 0..=32 - --> $DIR/core-traits-no-impls-length-33.rs:8:19 - | -LL | let mut set = HashSet::new(); - | ^^^^^^^^^^^^ the trait `std::array::LengthAtMost32` is not implemented for `[usize; 33]` - | - = note: required because of the requirements on the impl of `std::cmp::Eq` for `[usize; 33]` - = note: required by `std::collections::HashSet::::new` - error[E0369]: binary operation `==` cannot be applied to type `[usize; 33]` - --> $DIR/core-traits-no-impls-length-33.rs:15:19 + --> $DIR/core-traits-no-impls-length-33.rs:14:19 | LL | [0_usize; 33] == [1_usize; 33] | ------------- ^^ ------------- [usize; 33] @@ -35,7 +26,7 @@ LL | [0_usize; 33] == [1_usize; 33] = note: an implementation of `std::cmp::PartialEq` might be missing for `[usize; 33]` error[E0369]: binary operation `<` cannot be applied to type `[usize; 33]` - --> $DIR/core-traits-no-impls-length-33.rs:20:19 + --> $DIR/core-traits-no-impls-length-33.rs:19:19 | LL | [0_usize; 33] < [1_usize; 33] | ------------- ^ ------------- [usize; 33] @@ -45,7 +36,7 @@ LL | [0_usize; 33] < [1_usize; 33] = note: an implementation of `std::cmp::PartialOrd` might be missing for `[usize; 33]` error[E0277]: the trait bound `&[usize; 33]: std::iter::IntoIterator` is not satisfied - --> $DIR/core-traits-no-impls-length-33.rs:25:14 + --> $DIR/core-traits-no-impls-length-33.rs:24:14 | LL | for _ in &[0_usize; 33] { | ^^^^^^^^^^^^^^ the trait `std::iter::IntoIterator` is not implemented for `&[usize; 33]` @@ -57,7 +48,7 @@ LL | for _ in &[0_usize; 33] { <&'a mut [T] as std::iter::IntoIterator> = note: required by `std::iter::IntoIterator::into_iter` -error: aborting due to 6 previous errors +error: aborting due to 5 previous errors Some errors have detailed explanations: E0277, E0369. For more information about an error, try `rustc --explain E0277`. From 6bf2cc2229768faa8e86e0e8a9f5bd8ebfc817a2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 09:44:03 +1100 Subject: [PATCH 03/18] Avoid instantiating many `Parser` structs in `generic_extension`. Currently, every iteration of the main loop in `generic_extension` instantiates a `Parser`, which is expensive because `Parser` is a large type. Many of those instantiations are only used immutably, particularly for simple-but-repetitive macros of the sort seen in `html5ever` and PR 68836. This commit initializes a single "base" parser outside the loop, and then uses `Cow` to avoid cloning it except for the mutating iterations. This speeds up `html5ever` runs by up to 15%. --- src/librustc_expand/lib.rs | 1 + src/librustc_expand/mbe/macro_parser.rs | 38 ++++----------- src/librustc_expand/mbe/macro_rules.rs | 62 ++++++++++++++++++------- 3 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/librustc_expand/lib.rs b/src/librustc_expand/lib.rs index 4fe7c268c4f0b..f119c956ced04 100644 --- a/src/librustc_expand/lib.rs +++ b/src/librustc_expand/lib.rs @@ -1,3 +1,4 @@ +#![feature(cow_is_borrowed)] #![feature(crate_visibility_modifier)] #![feature(decl_macro)] #![feature(proc_macro_diagnostic)] diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index b14725fd731b1..78f22f3e443b1 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/src/librustc_expand/mbe/macro_parser.rs @@ -78,13 +78,11 @@ use crate::mbe::{self, TokenTree}; use rustc_ast_pretty::pprust; use rustc_parse::parser::{FollowedByType, Parser, PathStyle}; -use rustc_parse::Directory; use rustc_session::parse::ParseSess; use rustc_span::symbol::{kw, sym, Symbol}; use syntax::ast::{Ident, Name}; use syntax::ptr::P; use syntax::token::{self, DocComment, Nonterminal, Token}; -use syntax::tokenstream::TokenStream; use rustc_errors::{FatalError, PResult}; use rustc_span::Span; @@ -92,6 +90,7 @@ use smallvec::{smallvec, SmallVec}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; +use std::borrow::Cow; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::mem; use std::ops::{Deref, DerefMut}; @@ -613,28 +612,9 @@ fn inner_parse_loop<'root, 'tt>( Success(()) } -/// Use the given sequence of token trees (`ms`) as a matcher. Match the given token stream `tts` -/// against it and return the match. -/// -/// # Parameters -/// -/// - `sess`: The session into which errors are emitted -/// - `tts`: The tokenstream we are matching against the pattern `ms` -/// - `ms`: A sequence of token trees representing a pattern against which we are matching -/// - `directory`: Information about the file locations (needed for the black-box parser) -/// - `recurse_into_modules`: Whether or not to recurse into modules (needed for the black-box -/// parser) -pub(super) fn parse( - sess: &ParseSess, - tts: TokenStream, - ms: &[TokenTree], - directory: Option>, - recurse_into_modules: bool, -) -> NamedParseResult { - // Create a parser that can be used for the "black box" parts. - let mut parser = - Parser::new(sess, tts, directory, recurse_into_modules, true, rustc_parse::MACRO_ARGUMENTS); - +/// Use the given sequence of token trees (`ms`) as a matcher. Match the token +/// stream from the given `parser` against it and return the match. +pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> NamedParseResult { // A queue of possible matcher positions. We initialize it with the matcher position in which // the "dot" is before the first token of the first token tree in `ms`. `inner_parse_loop` then // processes all of these possible matcher positions and produces possible next positions into @@ -659,7 +639,7 @@ pub(super) fn parse( // parsing from the black-box parser done. The result is that `next_items` will contain a // bunch of possible next matcher positions in `next_items`. match inner_parse_loop( - sess, + parser.sess, &mut cur_items, &mut next_items, &mut eof_items, @@ -684,7 +664,7 @@ pub(super) fn parse( if eof_items.len() == 1 { let matches = eof_items[0].matches.iter_mut().map(|dv| Lrc::make_mut(dv).pop().unwrap()); - return nameize(sess, ms, matches); + return nameize(parser.sess, ms, matches); } else if eof_items.len() > 1 { return Error( parser.token.span, @@ -736,13 +716,13 @@ pub(super) fn parse( // If there are no possible next positions AND we aren't waiting for the black-box parser, // then there is a syntax error. else if bb_items.is_empty() && next_items.is_empty() { - return Failure(parser.token.take(), "no rules expected this token in macro call"); + return Failure(parser.token.clone(), "no rules expected this token in macro call"); } // Dump all possible `next_items` into `cur_items` for the next iteration. else if !next_items.is_empty() { // Now process the next token cur_items.extend(next_items.drain(..)); - parser.bump(); + parser.to_mut().bump(); } // Finally, we have the case where we need to call the black-box parser to get some // nonterminal. @@ -754,7 +734,7 @@ pub(super) fn parse( let match_cur = item.match_cur; item.push_match( match_cur, - MatchedNonterminal(Lrc::new(parse_nt(&mut parser, span, ident.name))), + MatchedNonterminal(Lrc::new(parse_nt(parser.to_mut(), span, ident.name))), ); item.idx += 1; item.match_cur += 1; diff --git a/src/librustc_expand/mbe/macro_rules.rs b/src/librustc_expand/mbe/macro_rules.rs index 29d41543fbf8c..9432790e78ced 100644 --- a/src/librustc_expand/mbe/macro_rules.rs +++ b/src/librustc_expand/mbe/macro_rules.rs @@ -1,11 +1,11 @@ -use crate::base::{DummyResult, ExtCtxt, MacResult, TTMacroExpander}; +use crate::base::{DummyResult, ExpansionData, ExtCtxt, MacResult, TTMacroExpander}; use crate::base::{SyntaxExtension, SyntaxExtensionKind}; use crate::expand::{ensure_complete_parse, parse_ast_fragment, AstFragment, AstFragmentKind}; use crate::mbe; use crate::mbe::macro_check; -use crate::mbe::macro_parser::parse; +use crate::mbe::macro_parser::parse_tt; use crate::mbe::macro_parser::{Error, Failure, Success}; -use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq, NamedParseResult}; +use crate::mbe::macro_parser::{MatchedNonterminal, MatchedSeq}; use crate::mbe::transcribe::transcribe; use rustc_ast_pretty::pprust; @@ -166,9 +166,9 @@ impl TTMacroExpander for MacroRulesMacroExpander { } } -fn trace_macros_note(cx: &mut ExtCtxt<'_>, sp: Span, message: String) { +fn trace_macros_note(cx_expansions: &mut FxHashMap>, sp: Span, message: String) { let sp = sp.macro_backtrace().last().map(|trace| trace.call_site).unwrap_or(sp); - cx.expansions.entry(sp).or_default().push(message); + cx_expansions.entry(sp).or_default().push(message); } /// Given `lhses` and `rhses`, this is the new macro we create @@ -184,12 +184,36 @@ fn generic_extension<'cx>( ) -> Box { if cx.trace_macros() { let msg = format!("expanding `{}! {{ {} }}`", name, pprust::tts_to_string(arg.clone())); - trace_macros_note(cx, sp, msg); + trace_macros_note(&mut cx.expansions, sp, msg); } // Which arm's failure should we report? (the one furthest along) let mut best_failure: Option<(Token, &str)> = None; + + // We create a base parser that can be used for the "black box" parts. + // Every iteration needs a fresh copy of that base parser. However, the + // parser is not mutated on many of the iterations, particularly when + // dealing with macros like this: + // + // macro_rules! foo { + // ("a") => (A); + // ("b") => (B); + // ("c") => (C); + // // ... etc. (maybe hundreds more) + // } + // + // as seen in the `html5ever` benchmark. We use a `Cow` so that the base + // parser is only cloned when necessary (upon mutation). Furthermore, we + // reinitialize the `Cow` with the base parser at the start of every + // iteration, so that any mutated parsers are not reused. This is all quite + // hacky, but speeds up the `html5ever` benchmark significantly. (Issue + // 68836 suggests a more comprehensive but more complex change to deal with + // this situation.) + let base_parser = base_parser_from_cx(&cx.current_expansion, &cx.parse_sess, arg.clone()); + for (i, lhs) in lhses.iter().enumerate() { + let mut parser = Cow::Borrowed(&base_parser); + // try each arm's matchers let lhs_tt = match *lhs { mbe::TokenTree::Delimited(_, ref delim) => &delim.tts[..], @@ -202,7 +226,7 @@ fn generic_extension<'cx>( // are not recorded. On the first `Success(..)`ful matcher, the spans are merged. let mut gated_spans_snaphot = mem::take(&mut *cx.parse_sess.gated_spans.spans.borrow_mut()); - match parse_tt(cx, lhs_tt, arg.clone()) { + match parse_tt(&mut parser, lhs_tt) { Success(named_matches) => { // The matcher was `Success(..)`ful. // Merge the gated spans from parsing the matcher with the pre-existing ones. @@ -232,7 +256,7 @@ fn generic_extension<'cx>( if cx.trace_macros() { let msg = format!("to `{}`", pprust::tts_to_string(tts.clone())); - trace_macros_note(cx, sp, msg); + trace_macros_note(&mut cx.expansions, sp, msg); } let directory = Directory { @@ -269,6 +293,7 @@ fn generic_extension<'cx>( // Restore to the state before snapshotting and maybe try again. mem::swap(&mut gated_spans_snaphot, &mut cx.parse_sess.gated_spans.spans.borrow_mut()); } + drop(base_parser); let (token, label) = best_failure.expect("ran no matchers"); let span = token.span.substitute_dummy(sp); @@ -286,7 +311,9 @@ fn generic_extension<'cx>( mbe::TokenTree::Delimited(_, ref delim) => &delim.tts[..], _ => continue, }; - match parse_tt(cx, lhs_tt, arg.clone()) { + let base_parser = + base_parser_from_cx(&cx.current_expansion, &cx.parse_sess, arg.clone()); + match parse_tt(&mut Cow::Borrowed(&base_parser), lhs_tt) { Success(_) => { if comma_span.is_dummy() { err.note("you might be missing a comma"); @@ -368,7 +395,8 @@ pub fn compile_declarative_macro( ), ]; - let argument_map = match parse(sess, body, &argument_gram, None, true) { + let base_parser = Parser::new(sess, body, None, true, true, rustc_parse::MACRO_ARGUMENTS); + let argument_map = match parse_tt(&mut Cow::Borrowed(&base_parser), &argument_gram) { Success(m) => m, Failure(token, msg) => { let s = parse_failure_msg(&token); @@ -1184,14 +1212,16 @@ fn quoted_tt_to_string(tt: &mbe::TokenTree) -> String { } } -/// Use this token tree as a matcher to parse given tts. -fn parse_tt(cx: &ExtCtxt<'_>, mtch: &[mbe::TokenTree], tts: TokenStream) -> NamedParseResult { - // `None` is because we're not interpolating +fn base_parser_from_cx<'cx>( + current_expansion: &'cx ExpansionData, + sess: &'cx ParseSess, + tts: TokenStream, +) -> Parser<'cx> { let directory = Directory { - path: Cow::from(cx.current_expansion.module.directory.as_path()), - ownership: cx.current_expansion.directory_ownership, + path: Cow::from(current_expansion.module.directory.as_path()), + ownership: current_expansion.directory_ownership, }; - parse(cx.parse_sess(), tts, mtch, Some(directory), true) + Parser::new(sess, tts, Some(directory), true, true, rustc_parse::MACRO_ARGUMENTS) } /// Generates an appropriate parsing failure message. For EOF, this is "unexpected end...". For From f840a955bd449810e75d8320b4c46482d6dbdec1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 14:33:08 +1100 Subject: [PATCH 04/18] Remove the `Cow` from `Directory`. The previous commit wrapped `Parser` within a `Cow` for the hot macro parsing path. As a result, there's no need for the `Cow` within `Directory`, because it lies within `Parser`. --- src/librustc_expand/mbe/macro_rules.rs | 4 ++-- src/librustc_parse/lib.rs | 9 ++++----- src/librustc_parse/parser/mod.rs | 9 ++++----- src/librustc_parse/parser/module.rs | 6 +++--- 4 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/librustc_expand/mbe/macro_rules.rs b/src/librustc_expand/mbe/macro_rules.rs index 9432790e78ced..9e6edee265c98 100644 --- a/src/librustc_expand/mbe/macro_rules.rs +++ b/src/librustc_expand/mbe/macro_rules.rs @@ -260,7 +260,7 @@ fn generic_extension<'cx>( } let directory = Directory { - path: Cow::from(cx.current_expansion.module.directory.as_path()), + path: cx.current_expansion.module.directory.clone(), ownership: cx.current_expansion.directory_ownership, }; let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), true, false, None); @@ -1218,7 +1218,7 @@ fn base_parser_from_cx<'cx>( tts: TokenStream, ) -> Parser<'cx> { let directory = Directory { - path: Cow::from(current_expansion.module.directory.as_path()), + path: current_expansion.module.directory.clone(), ownership: current_expansion.directory_ownership, }; Parser::new(sess, tts, Some(directory), true, true, rustc_parse::MACRO_ARGUMENTS) diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs index cd674e3c5ebef..4aad2c0f68a29 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -12,8 +12,7 @@ use syntax::ast; use syntax::token::{self, Nonterminal}; use syntax::tokenstream::{self, TokenStream, TokenTree}; -use std::borrow::Cow; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str; use log::info; @@ -29,8 +28,8 @@ pub mod validate_attr; pub mod config; #[derive(Clone)] -pub struct Directory<'a> { - pub path: Cow<'a, Path>, +pub struct Directory { + pub path: PathBuf, pub ownership: DirectoryOwnership, } @@ -274,7 +273,7 @@ pub fn stream_to_parser<'a>( pub fn stream_to_parser_with_base_dir<'a>( sess: &'a ParseSess, stream: TokenStream, - base_dir: Directory<'a>, + base_dir: Directory, ) -> Parser<'a> { Parser::new(sess, stream, Some(base_dir), true, false, None) } diff --git a/src/librustc_parse/parser/mod.rs b/src/librustc_parse/parser/mod.rs index 8c1839da1cb8f..cb95750d984e9 100644 --- a/src/librustc_parse/parser/mod.rs +++ b/src/librustc_parse/parser/mod.rs @@ -29,7 +29,6 @@ use syntax::token::{self, DelimToken, Token, TokenKind}; use syntax::tokenstream::{self, DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use syntax::util::comments::{doc_comment_style, strip_doc_comment_decoration}; -use std::borrow::Cow; use std::path::PathBuf; use std::{cmp, mem, slice}; @@ -114,7 +113,7 @@ pub struct Parser<'a> { prev_token_kind: PrevTokenKind, restrictions: Restrictions, /// Used to determine the path to externally loaded source files. - pub(super) directory: Directory<'a>, + pub(super) directory: Directory, /// `true` to parse sub-modules in other files. // Public for rustfmt usage. pub recurse_into_file_modules: bool, @@ -376,7 +375,7 @@ impl<'a> Parser<'a> { pub fn new( sess: &'a ParseSess, tokens: TokenStream, - directory: Option>, + directory: Option, recurse_into_file_modules: bool, desugar_doc_comments: bool, subparser_name: Option<&'static str>, @@ -390,7 +389,7 @@ impl<'a> Parser<'a> { restrictions: Restrictions::empty(), recurse_into_file_modules, directory: Directory { - path: Cow::from(PathBuf::new()), + path: PathBuf::new(), ownership: DirectoryOwnership::Owned { relative: None }, }, root_module_name: None, @@ -418,7 +417,7 @@ impl<'a> Parser<'a> { &sess.source_map().lookup_char_pos(parser.token.span.lo()).file.unmapped_path { if let Some(directory_path) = path.parent() { - parser.directory.path = Cow::from(directory_path.to_path_buf()); + parser.directory.path = directory_path.to_path_buf(); } } } diff --git a/src/librustc_parse/parser/module.rs b/src/librustc_parse/parser/module.rs index 6ce94d3c6793c..0c8fad03d8690 100644 --- a/src/librustc_parse/parser/module.rs +++ b/src/librustc_parse/parser/module.rs @@ -285,7 +285,7 @@ impl<'a> Parser<'a> { fn push_directory(&mut self, id: Ident, attrs: &[Attribute]) { if let Some(path) = attr::first_attr_value_str_by_name(attrs, sym::path) { - self.directory.path.to_mut().push(&*path.as_str()); + self.directory.path.push(&*path.as_str()); self.directory.ownership = DirectoryOwnership::Owned { relative: None }; } else { // We have to push on the current module name in the case of relative @@ -297,10 +297,10 @@ impl<'a> Parser<'a> { if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership { if let Some(ident) = relative.take() { // remove the relative offset - self.directory.path.to_mut().push(&*ident.as_str()); + self.directory.path.push(&*ident.as_str()); } } - self.directory.path.to_mut().push(&*id.as_str()); + self.directory.path.push(&*id.as_str()); } } } From 2a13b24d369b8619f0197993cd5dc60f7217ed72 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 5 Feb 2020 15:09:24 +1100 Subject: [PATCH 05/18] Change condition ordering in `parse_tt`. This is a small win, because `Failure` is much more common than `Success`. --- src/librustc_expand/mbe/macro_parser.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index 78f22f3e443b1..5bf7602ea6e8f 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/src/librustc_expand/mbe/macro_parser.rs @@ -689,9 +689,14 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na // unnecessary implicit clone later in Rc::make_mut. drop(eof_items); + // If there are no possible next positions AND we aren't waiting for the black-box parser, + // then there is a syntax error. + if bb_items.is_empty() && next_items.is_empty() { + return Failure(parser.token.clone(), "no rules expected this token in macro call"); + } // Another possibility is that we need to call out to parse some rust nonterminal // (black-box) parser. However, if there is not EXACTLY ONE of these, something is wrong. - if (!bb_items.is_empty() && !next_items.is_empty()) || bb_items.len() > 1 { + else if (!bb_items.is_empty() && !next_items.is_empty()) || bb_items.len() > 1 { let nts = bb_items .iter() .map(|item| match item.top_elts.get_tt(item.idx) { @@ -713,11 +718,6 @@ pub(super) fn parse_tt(parser: &mut Cow<'_, Parser<'_>>, ms: &[TokenTree]) -> Na ), ); } - // If there are no possible next positions AND we aren't waiting for the black-box parser, - // then there is a syntax error. - else if bb_items.is_empty() && next_items.is_empty() { - return Failure(parser.token.clone(), "no rules expected this token in macro call"); - } // Dump all possible `next_items` into `cur_items` for the next iteration. else if !next_items.is_empty() { // Now process the next token From a60669d95cdad0e28cf28790b717bbcf235153f8 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 9 Feb 2020 18:38:21 -0500 Subject: [PATCH 06/18] Properly use parent generics for opaque types Fixes #67844 Previously, opaque types would only get parent generics if they a return-position-impl-trait (e.g. `fn foo() -> impl MyTrait`). However, it's possible for opaque types to be nested inside one another: ```rust trait WithAssoc { type AssocType; } trait WithParam {} type Return = impl WithAssoc>; ``` When this occurs, we need to ensure that the nested opaque types properly inherit generic parameters from their parent opaque type. This commit fixes the `generics_of` query to take the parent item into account when determining the generics for an opaque type. --- src/librustc_typeck/collect.rs | 14 +++++++- .../issue-67844-nested-opaque.rs | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/type-alias-impl-trait/issue-67844-nested-opaque.rs diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 2a450f4b4e8b1..49b1bfb72a355 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1054,7 +1054,19 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { Some(tcx.closure_base_def_id(def_id)) } Node::Item(item) => match item.kind { - ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn, .. }) => impl_trait_fn, + ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn, .. }) => { + impl_trait_fn.or_else(|| { + let parent_id = tcx.hir().get_parent_item(hir_id); + // This opaque type might occur inside another opaque type + // (e.g. `impl Foo>`) + if parent_id != hir_id && parent_id != CRATE_HIR_ID { + debug!("generics_of: parent of opaque ty {:?} is {:?}", def_id, parent_id); + Some(tcx.hir().local_def_id(parent_id)) + } else { + None + } + }) + } _ => None, }, _ => None, diff --git a/src/test/ui/type-alias-impl-trait/issue-67844-nested-opaque.rs b/src/test/ui/type-alias-impl-trait/issue-67844-nested-opaque.rs new file mode 100644 index 0000000000000..2f844b4a05f5f --- /dev/null +++ b/src/test/ui/type-alias-impl-trait/issue-67844-nested-opaque.rs @@ -0,0 +1,32 @@ +// check-pass +// Regression test for issue #67844 +// Ensures that we properly handle nested TAIT occurences +// with generic parameters + +#![feature(type_alias_impl_trait)] + +trait WithAssoc { type AssocType; } + +trait WithParam {} + +type Return = impl WithAssoc>; + +struct MyParam; +impl WithParam for MyParam {} + +struct MyStruct; + +impl WithAssoc for MyStruct { + type AssocType = MyParam; +} + + +fn my_fun() -> Return { + MyStruct +} + +fn my_other_fn() -> impl WithAssoc> { + MyStruct +} + +fn main() {} From c18476e231058b8dd8528ca98b0b51ff14b729be Mon Sep 17 00:00:00 2001 From: ImgBotApp Date: Tue, 11 Feb 2020 03:01:20 +0000 Subject: [PATCH 07/18] [ImgBot] Optimize images *Total -- 10.65kb -> 8.44kb (20.82%) /src/etc/installer/gfx/rust-logo.png -- 5.71kb -> 3.82kb (33.11%) /src/librustdoc/html/static/down-arrow.svg -- 0.63kb -> 0.50kb (20.44%) /src/librustdoc/html/static/wheel.svg -- 3.86kb -> 3.68kb (4.66%) /src/librustdoc/html/static/brush.svg -- 0.47kb -> 0.44kb (4.61%) Signed-off-by: ImgBotApp --- src/etc/installer/gfx/rust-logo.png | Bin 5844 -> 3909 bytes src/librustdoc/html/static/brush.svg | 2 +- src/librustdoc/html/static/down-arrow.svg | 2 +- src/librustdoc/html/static/wheel.svg | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/etc/installer/gfx/rust-logo.png b/src/etc/installer/gfx/rust-logo.png index 2c3de3000879b36b1dc4751ea5db0fa8fce4b5aa..99ee7507fa2fb0d121ca340682204ca92a1b781e 100644 GIT binary patch literal 3909 zcmZ`+c{~$<`=0xjqg>@EN69rt!^)5&$$ceq*$pFG0DuX0QfD|> zPLxS(!-ErG^)s+A002H_^UyrmPI{SpH*75cfLH|pAQ1ro9G*ytYXCq55CGWl004Af z0sx{RuRCn6o(y!XZ`m84;K0DZVE-T*g&rRsog5v*jg9rA`%gG-baZlTY;1IN0*9L% z8Nm$?4-E}XjpL>#CK(I{z+l?(#4HeQYiS1n>I)=pkIIA~(b*`1hc)L&$jNd5lb5-v zkx*Rc!xIkR2sg7Z=2&LtVPO&ulU>g{Icm$y$iO~&cI(ZLq_>xO^$Uzy*+`6pzj}Ln zfpx6jJgPaX#UIHp2TX)g%FGK@?-u6>mVLBqGX$3139v9TwLu=gWqe0KGG{l3)nF6r zP9o(|I2Ehz4@DV1{I);GH%YH#S~FISDMb+@SDbIPNo813FVI@OURcil)Mt4}tw(!j zQ3Mx^&mp$MW3I`Xp-B^`YrzCaG7a8|t^V#@5$#{Wc}z+O9z6 zJEtoz`46#d?yMVK+s0b%$`o?9hALGZxy_a)Ec#&0e|IQ|+TxMWl;9Tmi_xFOUn+9w z8i#P1fw-OCn3OClbvelWgMrFO-bLq$d~$8!Jdbbe8M$|}d)*ug6R$kr7v5i2ay;L+ zX^+|&97y4{&@npS&CwT6xYmL2${|o0J^|n`yu8G42{%*!2_Z-U# z`wK7pEvhb>|Dya0EDt%vzQzz7{&vNpnnPXkLm6trvK2TrbIaAXM~{sUz>+d9Ig3Z7 zBbpa7|y(OWyk$ zeQgb;sZNM;H!e4cP)B*9VB?UqOJe#rX9$0KGy}A{ z>kDEQ9;xIel;97h^|r^xk@F1dLTJ%Y0cE)98MFk?sOiT*Ge3bX_xTVzTbdKmt{*Dg z<*djdbBnBu`y~DtVOmu(g#3yKRhcjPe<|xkN_B|(8ihGXg*+9AsJg(c{-B%2r0t~bHE9E~N*$8o5ZnIHU(CeFh>eQRGm^(0KyuAK`ma6~0OI2}E{OwpyPZxiDw&h#S+-F0+>BvA6Cc zX(jCBEZ$gtv)F$h&fV6N80CilSNfAQiUUNDt`B~o!LLbwEdfm2XC=Y zo|lSU`dpds_f~@PJxX?(9h`F0gG}CR{W7MWo$F^MgLiK31E$Il+H>QksmN&0yK$?Vr!^)nLulGtl}R$2 z8`LIM$?zcui)b<6g`{GBC~N2PgT23U=bSj(Omw1Bw|&k= ze$;As^8LZlHcH#yJsET?wJQD7RHs{smi7#Dtep~^u#kk?7oU4>$NVDw$7+P2;AF6d zTB?=PiZ%Dje7mBE;kS(|3Joycx-k5Pyn7_c&T{iEed1KRf7p1bG&pW0n zbT57GIHye*2L0zrof{FqSfwOxpg^oSRR2?wIr=k@2zfv37Ja{q(Fk4nqjqtZ*|(bw zCVM@YHv<{Qb)53>MS+sxis(1FAIncafNc*d8dmfpOkV~8+@H5S1@l`k`%5T%I#0a- zho`WtAEOS=Oc5*0NxO0h(%mA8fpTH>O@v08W}K1S>yGDA9KMyB&W$s+6|_j_*TI*} z)Pjft3t;DekOtuNjfTg_WIgMHuz-scYO4})wfxk0yj^J!Tc-4MsyiS)@poa3xZzUj zjD55eDL;lJ%QuHF%8*vxdEEA7r9R27K1&xzObU7dEL9YJx*|ru9QC~k8^7y7nrjyP zR+i!EK$^fhbmn5o$LJ@liO)fOUP9TKN+W%>N9;?EF`cKQIyCoWbe9ERpan93c`U%SBP~ za#8Q+X-p&Rh^@s&xvvS?;>MdqU)FI$3fhjbh40AB4W_G6gxXNe7s2UJ6E~M2Qq;g!sQ%dJo!OE z0n~{)nhE!Lgr>YXrTCNumvz+j*>+ZO3E}Fk=D{&}83Hpd9zV{`$t-?R>+XJUl2neT z?Kp-V$C0Q8aiZij`pRwvtc#|iBWnxSyKN*ld^)TkIC3~LyxeKMyR40+>*fL9wzk^D zv776SQuoW#Tj>d&4H>!vbsJY}A|#KE)D4wc3)CB{Sz(g`GzR2Sp$dcVboF)>esXWZ zct-`)Hv*lreUF{KL%u(9$mCr@t`yDNhxB>>88vUgxs2?^d0R9n{gQWCFJL=XC_~&8 z2&pWb-22=|(WJSs!|)`(rHkvMXLV_=w3_P`I`Ue_CT7mFxf|hOx+Z%NdCzZ(Su zLQOzv#L)gji=J%UvC6^yq`bgQ%0xJN$hPzTH3D&(lP0#{_eG(lZ-x}`zTygYq!}JM z+>@QAJ=mOev>h7A4j)R&{^&}e&ypCoi6K+&1J}+(QSX>)fk}@*8`Zr~jz24585ksl zmSEfx(&)}h$CDVKtC*E|2VVLNsb50M=E45w&G8W=iR+wGO`bED9SU4hY)K2;693H) zw}yg25hax_c>+^v5rcT=duCx27JKyVbmLevo!Jf1+6N`uB@a%~+cyRc(du}Tw2{Nj z4C9rulmd*o$Rjzs zw1vyPPyV*B!9%gHnXL7kPn}m56=;fAF&fh5u&)y|aQU@&7SskM$3QOpft*(H&Ol@$ z<=wYDbCQdYvUw383}u()%w$J3_X*DROE+gZg=AB>eYs;#BrKb@Ks?n8e3pSo$xaed zcCueSdoJCXGsKkI3w&6vN4uA_vh=iT;4zXvK`f`5kMYvuL zNVTR4q^yb=&6bPWhk(--X!Fu^PCx>G+MEs3D>N9o_L#y{Pbcw;8vE5M@>mygVAZ69 za5DLw(PwoMq+kx`=%lyV>|N!XmDp|dGL9b8FQmHCi`1{$6Yxxejbk1KEqm&ylFiCT zuf0Z~m7g)=PVA^0syY0wtQD2Pp9}1lPd+SnV6}J$h~;gU&IB$B>R|G~CCm zskB&kO2ei0KhMy=+D2)=DLf6uyR_s@-d8dRdPhH_CQ`=XpZYMLmmlU% zbdj=!qn}CR*c}WXhEBXE6>iUmI`W!u*B+%nH-X7#ww*yG#xkH1UK6dnbgaeQi@5xq8$DlElqev4f0ACmwVbb7@^ z81qGK0qqO?q+|Tbi>zN!sst0`!x^thd~V+*;5-+?cfG<=6-~-Xs+9QYF|-gpo#T>O z?XJK)FRX*Fw~KhTy_t!i9Nl&n_I`MvI1!a{7CSvFvUec#NS=B}`UxHURaf}Q6Xev? z2O@j9WcqI|280a1|6Ge8)QvYGvTL})t*A(rn6B5Z;&k|%uW@z7z=CGxKtyeFte)gH zS3=+tkSz($ME#U`&j@hHMtlWNT-0L3bPxjJ&wl@u+UitT+zzEDw+H=&g$Ro`Z27?h z&Dx7*v&^R(k_k5)CVYHQ$poWo(qojpyn6p$JLM(JF)w(Qa+>UZy({j#n=R~jixleF zn^=NnryT21_58%yjoXbIZk-#VbKz6BQNFoXTPHU%x_9KudK3ma_tKC5CG~MH2i0^9 ziT0qb7CqP8Pj6+TFNQgz;m&aiN-@jP%co~3&zeb2NRy$L2VYrFCo~NXr?|)Sg9B!y z)Dpk;kIhoXD~K%04U2<#67mu>^~tDGFEk#T@R#|%;-rPZCLpJLFYel)6Ce? KsKL-9>Hh%dF3!mS literal 5844 zcmX9i2RNJG*D)(piIQ41OVy@D6+vmyD5bU4ioI1ALWmWsswEUftE%=49W+w2s`j2i zjWkG95Cs4HzVCgW`@Hv_bMHC#-gE9*N%zc*IN1f+0RRA}iLs$20KfpJH$FC2I%HYc z$rbFOp;Van{5LwaR7$uM7x;auBRcz$oM%`usYPWb;ncie#PDQ)4DN)Sby>%t0w z0MSY0ZIb9IWniF`3!Cp0bzF#uI@niR-e1BBNCBdO9Vfqj{UTrnv7R(jS|x}Hz;C;8 zy6=po9qHiG3&jrsfLIAfH4^aH?-2(g(>el2rpgf((Zz+ibY)vb@O1xvq{Nu2-yiJBI6^gukxp+DzHfQPpESt?IGZ5IXn&uw99*tIRSVsL z+U`|x$pCVg(;F|Xdl0hgQ@xI8_J)8qNXnO+~c=!RveHst84V0pmgzmuPeIrlC zah?Ev?u-Q6`+CsilMG7dyia=Gj_-r-zyYp+AJJ1Tm5 zd^Z^8ZcQ;O^&?1V${sG(uS-#lW~DF+N4bih63?P`a$jH~TgPFW9J1{^&ufV8D3h9s z=V|^Kg1^UonX??2Z;^Zw)Nei=jnJrds1th#aRom6IiEHSyV__Rq&>Z&BHs=M|0@vc z)v^9NUtKI&@_Js?w-Fg#5)ve}?#a;Xo4~g;_UqQ8^!036Hn}C$sVg6K#?GmdR6Mr`GpFk~pmL^G>1bCKXs~L?n%aW{ zFY~qZZ}mu1f)09{{VRwAg)2K__8KK;SpqNSuu8F1bn&$sF`R#4r)>R;C1>-ZYqY`E zD?tc}2lu(Mn#Z3QKAmD4s^-RHuitj<(^#pF`u31E(fW6HEyCI&31eBItaM^_hoN24 za?Ce-P!zcCX%gC@?eigMjuG0*SGmt+n$`pH9xlhnLev9ozV9eAwaeO3ISvx#y z6rofkq!PdWWSAK0io|b*ZNWHO#+qOpeS3$Fyi5tV6KF(oT9{ZcLDH945nh zHl?-*#l95Pojt?@n#h8m<95)&qmY(@8;gZl zwn$ucZ6(d*m}E46N~two@N_Xk`p*!}x*k;~+JC=*QaJvy)MPAvxv^ivc}bE(iMjjZ z=7>{shrwJPM|8J>~X!mY$m%6Rcko7A7Y5{OW4=$@7eL zcMEu=sD9LiW|!eAiaZ3rtq_5ZdQZP}rMWvpvy62LYeqGNos88EjS%pRw5o#(N|~}f zHBpUv3?s_Z7WQU{dj9FhAM@7U3#jL@++6?GJQ7*nDst0(cJ>h)f)pl3O<{+uaz~@z zI53L>A-L`&D7mHzj!b7R#KSJ*g64j|RQf--nZkMdniSLp{s^SGI1r*p<}ET6dYs`w9oXN(I<--eRQLTEbhY_9+X;@I_r+gxa_<83>sl+K{i@9_F(3=NgJjZ|$=Gv>P|S zcWv3jR9J?-nUh+s*Bww7uXm}}x`GoOu^K|yYVo3yTc`gr=Lr;8RY>^pxgc6Z>hG0@ ze|9FrTvPB&xPdF=?bUTV;dV^D(c|Sc-;39zX0h z*`~O%`G4qV#-^}{R-?F!jT+pS2F7_tss8Cv)$3uR3pH6Dh<*LWi-17D=0s%d->vI^ zV4P6{W6xB4`*%*)1sRc8hfhD|?hOZ6whtrTmSRh<%f(Ta?v$-V7a-<8(+=+nLuA&h8uZb;YpG{rpCcn5H$3nih-E1+{1EX1TccBHMm}}% zd6C(!RbS%#ZOnkPV>)N0NQzqf0gq@{)lJQ<(JBX|N$=G>gkVPQ)5T8vVR1CiFv+KI z3{p7SXKdAp9QllxD{Fgrk8B|9l{!9g#|t(^YAg#HbFCV0U1jBRu4% zyosZUU}!Bep3iUNRfa^3?p#`do4GtSZ!mqM5b=O=6?k{EsgH0>i_?Xw^Pe84BC+oW zy_zUCB#)ca&|viQi@3{_D+NkTO zIDR$Z9$%gf5&QMmr(Q%771+tCNHr26*;-JHyi>kOxN#Dv?%R+LNN2z_ZeAAN1Nq~K zf=h9_OT$e#CIzc;srFz!-05oR#HL~yo%}B`_K`*}#hDwO<$}g!4N+m0pFV_!4I*{l zD{|kZ^BI`!bB%Vk!B{lRfD60SHD9~Z(tt1VuyeTu|4OeBrn-0#asy{qM%xy`g0GF zqQvQB6KU#92Y1Hz$@!4h2QdW-r)i5#NeiKB{c1LKUZ0=0xN4>1LpcGNb$dYU8S`Nd zh_g;xh_+zLHXe8l1h-L${)f`(9Nx#92;vryQE(jKNw7(?4oT$)3HN#nS)+HR2=aK5 zmPXru%dCsMQ$7cncH1q`w$_8;)_ySHmk5@@C9d`nxNHZmm+hSmZt;quQIfb*bUdt< zfk%Ark1rZx^XL}N#+c=$kQ|?HKzK)qqY3$d$nX!gL#&|ydRwbks0Tg&4n&rlC^}{V zb{w)+11->wcEC z0N_s@MHg`*7gFs-@Z$jj%D8dHTot1rxHSau!62#m#dexlb^lwyOFK#u#+nOI3$VU3 z4DIFI0Mt?}%tZk%gyr1`F>4Tgi1A^i4+n;;?x(BJJH!$uK-=(y zB?CY#W4)rM_3pS1z?6yaOS3?0QY8-{$Bbfkm_q4S!CfaNaNf>tH?H9DOViy8=$PcD zEz}hHR7IRm{lzy7*J4}Rv`lOZ8Z;cUbKqxc+s+tex$ira0A7Fpj>uH-7rDoG9Wly& zN$TwCs!PW|xY77N>(gXHF7%p7LkuW=*Zz$aRT9ui;ygSSWkx-PN zfxhKT4Lg^cb$iq3#Zk`* z&5pScdIprItQyAJKY+;f5V;M310hQ;;MGrryzMEkT1qMDr3o1n3N-s{a(mQ*0F zddm{$AaqW}2t0s#FS_;eO4liTJt0pP`T72{zQ#pvyIdn>2V(pt;CuM_u!7jO+Xj7| zLx#F>!K@SgI`NM!(z(-Cmc;mFe~+^oxD!G*Gj-V9v@O4``zsrwAroeuVel}`&dCn_ z!DDV^ag=uC;9wZe;$NM#@C+imCu>&7B-*8@EdS3k&6iyh}W8LEQX=WA9@GE8YBxE~d zOdN<#G&)nq2{Y@u^!6TMsmc%PWK{jrGF`Fr2Jvs2VcV%qM zhQqEhpxR4!OX~kg7MYTRIHH=VDL_48>}4oe&KYO*k7=4&?B5CzJym_#assFdLcxD@ zTB;NlVsl@U7COG_CVES7uep%AEqU#w%0Ed_QLj0oyq?9}tPrZsA0c$}f}Wg5UQe&3 zgv;D|HzwAaZp!vvM2o#jnvi8c2JL!Uc#kPW{3RrsDyyL6@5Ed4Icf?x%Uo4KjqvSV zYmnu$PaFA%#7p}2mJF67@ukMNOH`B}FIA@TZ}({WNRF5x;SHUb&s#k*=mKv6t5q|5 z)yeaQJ6TA&lBnIk@2wV9n~#@VI`v9=UVyv;4Vl~ ztg!*X4J$XHiVwXpB`OIo7e?-?o{K)q_r1*iGF@HOH1E8#n!Wv^k@H2Ko_UIHWDy#l ztu+O!tG?ssqJvYi^R}wr)q_{p?nN+XET2xb4(Q;5cs?AE+m}%BA_C%dQ%itMOh%%b z;V1WZR%xd1MHCA%bwb%7xS$wvMOT)yz8PZtxy-kYmhpaxWPmTUz4+G4e7f7DpiTssZ{$0=EPkNsG<*p;9SN1Br(9B8H4&Q))> znX!iNtcay2_BM<`Bv2gmY5?~{ml#zc5ibJLc^%`?xfNiH=`?#5Or3Z7eTJ& zD*-9@zIs(eRT8fi34UX7Y2 zcK|nXLCB)=7C98mnAPV+TI}L4OWS#RFY#BYy>p~c96of@bW))~?+`|i&lUkkrNe2X zilTJ5SKYs=GRM}3cS;AIJ|e(#Tp{f%_uXm!+{l4!`Wq2_0vrvFr+hMeOIYRK38%(9 zrmumH3mqktv{RJ+-xnB{g^-@8_sV^tez z=&9@e3(lM51~IS-jD%MC~BUP$+R5?9lT#y9^Ja;|sr zFArO`=8umf6|kOp(6IS2d;C(EJHFd%FVF<8jeI#EmNSnhE2%-u4mhTWfi66t(Z`0AV0$XwC@Qx>w zBb`k^hd&$L&t~`&^=b3v#1iAa_5^XNPA3Fhhk0zu@7J2sn9K5Z+QUY+EkF+##npj% zU~RT#&5}Jc5%;?r-MB%|Oi1wx#XkgZtnUO4esn2m*{j4o=PKf+x!zYY_)z&)wL0J0 zoVEJZ0>nzl{XY%%Tr~bB+)VuQ)HW2Cmgfnj-`8p{eWs*9x z+U%26L~7FzvuK7G@0Z1g?8VH_L~r!u{jOp#%KvOYsuV`qr zjbNbSg|{(AeSQpkeLWdytqc!yy0MWhGhbbE*6f*i;uIj?k(B2lwYyW3Gf?Q}6rgMf zc2ROXlx=N+JL*&K_f>Ev^e}FuLH9>Zb`JcS>!j&M*mJp3W-qAm4^=Mo?dgCKd}xP>MRX1kQjfqNS??x zVq5Krx1UL?DT^3LFsy8TS8T`cBTdzaJQC5<7oLsZq0!WC35wuEWj{F_OMAykpgkzH z+3_Q+SNGewV*!7m+$xhnMr!I(=Mn{dc?n#=c}m0 zl$tX!oY&c#-u^59`*MZuf_?eEwdx>BHW;I~E|eL7e&f*wQnQhy9-pIp-0{PwKg9i5 zP`?2iddTSN6=L`0fli(IeiGhXnj-b+FEHu|-<$g7cg96-pO}m*c!05s?MB!n63-P1 zBbE!P^Ww1aY!VLx0c-KT{YSvgb*>V6V5-eH7fYt&7W@vn_oZmaW@ZE)E%4uD8*2Mp z``E}lpB}l}^FpBxJ-X<0)Ol@v!_y \ No newline at end of file + \ No newline at end of file diff --git a/src/librustdoc/html/static/down-arrow.svg b/src/librustdoc/html/static/down-arrow.svg index c0f59f0c36fce..35437e77a710c 100644 --- a/src/librustdoc/html/static/down-arrow.svg +++ b/src/librustdoc/html/static/down-arrow.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file diff --git a/src/librustdoc/html/static/wheel.svg b/src/librustdoc/html/static/wheel.svg index 44381a401a82f..01da3b24c7c4f 100644 --- a/src/librustdoc/html/static/wheel.svg +++ b/src/librustdoc/html/static/wheel.svg @@ -1 +1 @@ - \ No newline at end of file + \ No newline at end of file From 34cf0b32674da79403746716e5a7ed2072dfabe2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 10 Feb 2020 23:35:49 -0500 Subject: [PATCH 08/18] Only use the parent if it's an opaque type --- src/librustc_typeck/collect.rs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 49b1bfb72a355..242faebe53ec8 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1057,11 +1057,19 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn, .. }) => { impl_trait_fn.or_else(|| { let parent_id = tcx.hir().get_parent_item(hir_id); - // This opaque type might occur inside another opaque type - // (e.g. `impl Foo>`) if parent_id != hir_id && parent_id != CRATE_HIR_ID { debug!("generics_of: parent of opaque ty {:?} is {:?}", def_id, parent_id); - Some(tcx.hir().local_def_id(parent_id)) + // If this 'impl Trait' is nested inside another 'impl Trait' + // (e.g. `impl Foo>`), we need to use the 'parent' + // 'impl Trait' for its generic parameters, since we can reference them + // from the 'child' 'impl Trait' + if let Node::Item(hir::Item { kind: ItemKind::OpaqueTy(..), .. }) = + tcx.hir().get(parent_id) + { + Some(tcx.hir().local_def_id(parent_id)) + } else { + None + } } else { None } From ad7802f9d45b884dad58931c7a8bec91d196ad0e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 10 Feb 2020 18:40:02 +1100 Subject: [PATCH 09/18] Micro-optimize the heck out of LEB128 reading and writing. This commit makes the following writing improvements: - Removes the unnecessary `write_to_vec` function. - Reduces the number of conditions per loop from 2 to 1. - Avoids a mask and a shift on the final byte. And the following reading improvements: - Removes an unnecessary type annotation. - Fixes a dangerous unchecked slice access. Imagine a slice `[0x80]` -- the current code will read past the end of the slice some number of bytes. The bounds check at the end will subsequently trigger, unless something bad (like a crash) happens first. The cost of doing bounds check in the loop body is negligible. - Avoids a mask on the final byte. And the following improvements for both reading and writing: - Changes `for` to `loop` for the loops, avoiding an unnecessary condition on each iteration. This also removes the need for `leb128_size`. All of these changes give significant perf wins, up to 5%. --- src/libserialize/leb128.rs | 64 +++++++++----------------------------- 1 file changed, 14 insertions(+), 50 deletions(-) diff --git a/src/libserialize/leb128.rs b/src/libserialize/leb128.rs index b8242f7154ee4..1fe6a309e9650 100644 --- a/src/libserialize/leb128.rs +++ b/src/libserialize/leb128.rs @@ -1,46 +1,14 @@ -#[inline] -pub fn write_to_vec(vec: &mut Vec, byte: u8) { - vec.push(byte); -} - -#[cfg(target_pointer_width = "32")] -const USIZE_LEB128_SIZE: usize = 5; -#[cfg(target_pointer_width = "64")] -const USIZE_LEB128_SIZE: usize = 10; - -macro_rules! leb128_size { - (u16) => { - 3 - }; - (u32) => { - 5 - }; - (u64) => { - 10 - }; - (u128) => { - 19 - }; - (usize) => { - USIZE_LEB128_SIZE - }; -} - macro_rules! impl_write_unsigned_leb128 { ($fn_name:ident, $int_ty:ident) => { #[inline] pub fn $fn_name(out: &mut Vec, mut value: $int_ty) { - for _ in 0..leb128_size!($int_ty) { - let mut byte = (value & 0x7F) as u8; - value >>= 7; - if value != 0 { - byte |= 0x80; - } - - write_to_vec(out, byte); - - if value == 0 { + loop { + if value < 0x80 { + out.push(value as u8); break; + } else { + out.push(((value & 0x7f) | 0x80) as u8); + value >>= 7; } } } @@ -57,24 +25,20 @@ macro_rules! impl_read_unsigned_leb128 { ($fn_name:ident, $int_ty:ident) => { #[inline] pub fn $fn_name(slice: &[u8]) -> ($int_ty, usize) { - let mut result: $int_ty = 0; + let mut result = 0; let mut shift = 0; let mut position = 0; - - for _ in 0..leb128_size!($int_ty) { - let byte = unsafe { *slice.get_unchecked(position) }; + loop { + let byte = slice[position]; position += 1; - result |= ((byte & 0x7F) as $int_ty) << shift; if (byte & 0x80) == 0 { - break; + result |= (byte as $int_ty) << shift; + return (result, position); + } else { + result |= ((byte & 0x7F) as $int_ty) << shift; } shift += 7; } - - // Do a single bounds check at the end instead of for every byte. - assert!(position <= slice.len()); - - (result, position) } }; } @@ -116,7 +80,7 @@ where #[inline] pub fn write_signed_leb128(out: &mut Vec, value: i128) { - write_signed_leb128_to(value, |v| write_to_vec(out, v)) + write_signed_leb128_to(value, |v| out.push(v)) } #[inline] From 1f6fb338a5f775745595d32b61c1862887c948f9 Mon Sep 17 00:00:00 2001 From: Dario Gonzalez Date: Tue, 11 Feb 2020 10:11:58 -0800 Subject: [PATCH 10/18] make the sgx arg cleanup implementation a no op --- src/libstd/sys/sgx/args.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/libstd/sys/sgx/args.rs b/src/libstd/sys/sgx/args.rs index b47a48e752cb7..5a53695a8466b 100644 --- a/src/libstd/sys/sgx/args.rs +++ b/src/libstd/sys/sgx/args.rs @@ -22,12 +22,7 @@ pub unsafe fn init(argc: isize, argv: *const *const u8) { } } -pub unsafe fn cleanup() { - let args = ARGS.swap(0, Ordering::Relaxed); - if args != 0 { - drop(Box::::from_raw(args as _)) - } -} +pub unsafe fn cleanup() {} pub fn args() -> Args { let args = unsafe { (ARGS.load(Ordering::Relaxed) as *const ArgsStore).as_ref() }; From 24be307b53931a9824829f63aa65fa5c6042ed21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 10 Feb 2020 19:14:31 -0800 Subject: [PATCH 11/18] Suggestion when encountering assoc types from hrtb When encountering E0212, detect whether this is a representable case or not, i.e. if it's happening on an `fn` or on an ADT. If the former, provide a structured suggestion, otherwise note that this can't be represented in Rust. --- src/librustc_typeck/collect.rs | 55 +++++++++++++++---- ...ciated-types-project-from-hrtb-in-fn.fixed | 37 +++++++++++++ ...ssociated-types-project-from-hrtb-in-fn.rs | 2 + ...iated-types-project-from-hrtb-in-fn.stderr | 4 +- ...es-project-from-hrtb-in-trait-method.fixed | 38 +++++++++++++ ...types-project-from-hrtb-in-trait-method.rs | 15 +++++ ...s-project-from-hrtb-in-trait-method.stderr | 12 +++- 7 files changed, 146 insertions(+), 17 deletions(-) create mode 100644 src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.fixed create mode 100644 src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.fixed diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 2a450f4b4e8b1..8d2b6512cfe46 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -278,6 +278,17 @@ impl ItemCtxt<'tcx> { pub fn to_ty(&self, ast_ty: &'tcx hir::Ty<'tcx>) -> Ty<'tcx> { AstConv::ast_ty_to_ty(self, ast_ty) } + + pub fn hir_id(&self) -> hir::HirId { + self.tcx + .hir() + .as_local_hir_id(self.item_def_id) + .expect("Non-local call to local provider is_const_fn") + } + + pub fn node(&self) -> hir::Node<'tcx> { + self.tcx.hir().get(self.hir_id()) + } } impl AstConv<'tcx> for ItemCtxt<'tcx> { @@ -290,15 +301,7 @@ impl AstConv<'tcx> for ItemCtxt<'tcx> { } fn default_constness_for_trait_bounds(&self) -> ast::Constness { - // FIXME: refactor this into a method - let hir_id = self - .tcx - .hir() - .as_local_hir_id(self.item_def_id) - .expect("Non-local call to local provider is_const_fn"); - - let node = self.tcx.hir().get(hir_id); - if let Some(fn_like) = FnLikeNode::from_node(node) { + if let Some(fn_like) = FnLikeNode::from_node(self.node()) { fn_like.constness() } else { ast::Constness::NotConst @@ -352,14 +355,42 @@ impl AstConv<'tcx> for ItemCtxt<'tcx> { self.tcx().mk_projection(item_def_id, item_substs) } else { // There are no late-bound regions; we can just ignore the binder. - struct_span_err!( + let mut err = struct_span_err!( self.tcx().sess, span, E0212, "cannot extract an associated type from a higher-ranked trait bound \ in this context" - ) - .emit(); + ); + + match self.node() { + hir::Node::Field(_) + | hir::Node::Variant(_) + | hir::Node::Ctor(_) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(..), .. }) => { + // The suggestion is only valid if this is not an ADT. + } + hir::Node::Item(_) + | hir::Node::ForeignItem(_) + | hir::Node::TraitItem(_) + | hir::Node::ImplItem(_) => { + err.span_suggestion( + span, + "use a fully qualified path with inferred lifetimes", + format!( + "{}::{}", + // Erase named lt, we want `::C`, not `::C`. + self.tcx.anonymize_late_bound_regions(&poly_trait_ref).skip_binder(), + item_segment.ident + ), + Applicability::MaybeIncorrect, + ); + } + _ => {} + } + err.emit(); self.tcx().types.err } } diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.fixed b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.fixed new file mode 100644 index 0000000000000..760d2b433c87a --- /dev/null +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.fixed @@ -0,0 +1,37 @@ +#![allow(dead_code, unused_variables)] +// run-rustfix +// Check projection of an associated type out of a higher-ranked trait-bound +// in the context of a function signature. + +pub trait Foo { + type A; + + fn get(&self, t: T) -> Self::A; +} + +fn foo2 Foo<&'x isize>>( + x: >::A) + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context +{ + // This case is illegal because we have to instantiate `'x`, and + // we don't know what region to instantiate it with. + // + // This could perhaps be made equivalent to the examples below, + // specifically for fn signatures. +} + +fn foo3 Foo<&'x isize>>( + x: >::A) +{ + // OK, in this case we spelled out the precise regions involved, though we left one of + // them anonymous. +} + +fn foo4<'a, I : for<'x> Foo<&'x isize>>( + x: >::A) +{ + // OK, in this case we spelled out the precise regions involved. +} + + +pub fn main() {} diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.rs b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.rs index bf13c410cc60b..6eb584ea645ac 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.rs +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.rs @@ -1,3 +1,5 @@ +#![allow(dead_code, unused_variables)] +// run-rustfix // Check projection of an associated type out of a higher-ranked trait-bound // in the context of a function signature. diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.stderr b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.stderr index 09f4f99703f9c..f2137f68665db 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.stderr +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-fn.stderr @@ -1,8 +1,8 @@ error[E0212]: cannot extract an associated type from a higher-ranked trait bound in this context - --> $DIR/associated-types-project-from-hrtb-in-fn.rs:11:8 + --> $DIR/associated-types-project-from-hrtb-in-fn.rs:13:8 | LL | x: I::A) - | ^^^^ + | ^^^^ help: use a fully qualified path with inferred lifetimes: `>::A` error: aborting due to previous error diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.fixed b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.fixed new file mode 100644 index 0000000000000..acf32bccbecfd --- /dev/null +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.fixed @@ -0,0 +1,38 @@ +#![allow(dead_code)] +// run-rustfix +// Check projection of an associated type out of a higher-ranked trait-bound +// in the context of a method definition in a trait. + +pub trait Foo { + type A; + + fn get(&self, t: T) -> Self::A; +} + +trait SomeTrait Foo<&'x isize>> { + fn some_method(&self, arg: >::A); + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context +} + +trait AnotherTrait Foo<&'x isize>> { + fn some_method(&self, arg: >::A); +} + +trait YetAnotherTrait Foo<&'x isize>> { + fn some_method<'a>(&self, arg: >::A); +} + +trait Banana<'a> { + type Assoc: Default; +} + +struct Peach(std::marker::PhantomData); + +impl Banana<'a>> Peach { + fn mango(&self) -> >::Assoc { + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context + Default::default() + } +} + +pub fn main() {} diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.rs b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.rs index cb52c2b4f15d6..a249f89685e39 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.rs +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] +// run-rustfix // Check projection of an associated type out of a higher-ranked trait-bound // in the context of a method definition in a trait. @@ -20,4 +22,17 @@ trait YetAnotherTrait Foo<&'x isize>> { fn some_method<'a>(&self, arg: >::A); } +trait Banana<'a> { + type Assoc: Default; +} + +struct Peach(std::marker::PhantomData); + +impl Banana<'a>> Peach { + fn mango(&self) -> X::Assoc { + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context + Default::default() + } +} + pub fn main() {} diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.stderr b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.stderr index e1c169028c5c4..a37fec244933c 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.stderr +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-trait-method.stderr @@ -1,8 +1,14 @@ error[E0212]: cannot extract an associated type from a higher-ranked trait bound in this context - --> $DIR/associated-types-project-from-hrtb-in-trait-method.rs:11:32 + --> $DIR/associated-types-project-from-hrtb-in-trait-method.rs:13:32 | LL | fn some_method(&self, arg: I::A); - | ^^^^ + | ^^^^ help: use a fully qualified path with inferred lifetimes: `>::A` -error: aborting due to previous error +error[E0212]: cannot extract an associated type from a higher-ranked trait bound in this context + --> $DIR/associated-types-project-from-hrtb-in-trait-method.rs:32:24 + | +LL | fn mango(&self) -> X::Assoc { + | ^^^^^^^^ help: use a fully qualified path with inferred lifetimes: `>::Assoc` + +error: aborting due to 2 previous errors From bde96776a199064dec3c825ca5ada8f90e1e12d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Feb 2020 12:37:12 -0800 Subject: [PATCH 12/18] Suggest named lifetime in ADT with hrtb --- src/librustc_typeck/collect.rs | 52 ++++++++++++++++--- ...iated-types-project-from-hrtb-in-struct.rs | 11 +++- ...d-types-project-from-hrtb-in-struct.stderr | 34 +++++++++++- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 8d2b6512cfe46..59077c2bc1257 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -364,14 +364,52 @@ impl AstConv<'tcx> for ItemCtxt<'tcx> { ); match self.node() { - hir::Node::Field(_) - | hir::Node::Variant(_) - | hir::Node::Ctor(_) - | hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(..), .. }) - | hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(..), .. }) - | hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(..), .. }) => { - // The suggestion is only valid if this is not an ADT. + hir::Node::Field(_) | hir::Node::Ctor(_) | hir::Node::Variant(_) => { + let item = + self.tcx.hir().expect_item(self.tcx.hir().get_parent_item(self.hir_id())); + match &item.kind { + hir::ItemKind::Enum(_, generics) + | hir::ItemKind::Struct(_, generics) + | hir::ItemKind::Union(_, generics) => { + // FIXME: look for an appropriate lt name if `'a` is already used + let (lt_sp, sugg) = match &generics.params[..] { + [] => (generics.span, "<'a>".to_string()), + [bound, ..] => (bound.span.shrink_to_lo(), "'a, ".to_string()), + }; + let suggestions = vec![ + (lt_sp, sugg), + ( + span, + format!( + "{}::{}", + // Replace the existing lifetimes with a new named lifetime. + self.tcx + .replace_late_bound_regions(&poly_trait_ref, |_| { + self.tcx.mk_region(ty::ReEarlyBound( + ty::EarlyBoundRegion { + def_id: item_def_id, + index: 0, + name: Symbol::intern("'a"), + }, + )) + }) + .0, + item_segment.ident + ), + ), + ]; + err.multipart_suggestion( + "use a fully qualified path with explicit lifetimes", + suggestions, + Applicability::MaybeIncorrect, + ); + } + _ => {} + } } + hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Enum(..), .. }) + | hir::Node::Item(hir::Item { kind: hir::ItemKind::Union(..), .. }) => {} hir::Node::Item(_) | hir::Node::ForeignItem(_) | hir::Node::TraitItem(_) diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.rs b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.rs index 20f11ecf63823..8a5777d4d7cb5 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.rs +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.rs @@ -7,18 +7,25 @@ pub trait Foo { fn get(&self, t: T) -> Self::A; } -struct SomeStruct Foo<&'x isize>> { +struct SomeStruct Foo<&'x isize>> { field: I::A //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context } +enum SomeEnum Foo<&'x isize>> { + TupleVariant(I::A), + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context + StructVariant { field: I::A }, + //~^ ERROR cannot extract an associated type from a higher-ranked trait bound in this context +} + // FIXME(eddyb) This one doesn't even compile because of the unsupported syntax. // struct AnotherStruct Foo<&'x isize>> { // field: Foo<&'y isize>>::A // } -struct YetAnotherStruct<'a, I : for<'x> Foo<&'x isize>> { +struct YetAnotherStruct<'a, I: for<'x> Foo<&'x isize>> { field: >::A } diff --git a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.stderr b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.stderr index 189b19461f479..c71bc70ea6c4e 100644 --- a/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.stderr +++ b/src/test/ui/associated-types/associated-types-project-from-hrtb-in-struct.stderr @@ -3,6 +3,38 @@ error[E0212]: cannot extract an associated type from a higher-ranked trait bound | LL | field: I::A | ^^^^ + | +help: use a fully qualified path with explicit lifetimes + | +LL | struct SomeStruct<'a, I: for<'x> Foo<&'x isize>> { +LL | field: >::A + | + +error[E0212]: cannot extract an associated type from a higher-ranked trait bound in this context + --> $DIR/associated-types-project-from-hrtb-in-struct.rs:16:18 + | +LL | TupleVariant(I::A), + | ^^^^ + | +help: use a fully qualified path with explicit lifetimes + | +LL | enum SomeEnum<'a, I: for<'x> Foo<&'x isize>> { +LL | TupleVariant(>::A), + | + +error[E0212]: cannot extract an associated type from a higher-ranked trait bound in this context + --> $DIR/associated-types-project-from-hrtb-in-struct.rs:18:28 + | +LL | StructVariant { field: I::A }, + | ^^^^ + | +help: use a fully qualified path with explicit lifetimes + | +LL | enum SomeEnum<'a, I: for<'x> Foo<&'x isize>> { +LL | TupleVariant(I::A), +LL | +LL | StructVariant { field: >::A }, + | -error: aborting due to previous error +error: aborting due to 3 previous errors From 33e2c1d863f53f5224db5abd40c6a84879051ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 12 Feb 2020 00:00:00 +0000 Subject: [PATCH 13/18] bootstrap: Configure cmake when building sanitizer runtimes --- src/bootstrap/native.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 1cfb4b2f63b57..566984a96fe09 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -262,7 +262,7 @@ impl Step for Llvm { cfg.define("PYTHON_EXECUTABLE", python); } - configure_cmake(builder, target, &mut cfg); + configure_cmake(builder, target, &mut cfg, true); // FIXME: we don't actually need to build all LLVM tools and all LLVM // libraries here, e.g., we just want a few components and a few @@ -301,7 +301,12 @@ fn check_llvm_version(builder: &Builder<'_>, llvm_config: &Path) { panic!("\n\nbad LLVM version: {}, need >=7.0\n\n", version) } -fn configure_cmake(builder: &Builder<'_>, target: Interned, cfg: &mut cmake::Config) { +fn configure_cmake( + builder: &Builder<'_>, + target: Interned, + cfg: &mut cmake::Config, + use_compiler_launcher: bool, +) { // Do not print installation messages for up-to-date files. // LLVM and LLD builds can produce a lot of those and hit CI limits on log size. cfg.define("CMAKE_INSTALL_MESSAGE", "LAZY"); @@ -372,9 +377,11 @@ fn configure_cmake(builder: &Builder<'_>, target: Interned, cfg: &mut cm } else { // If ccache is configured we inform the build a little differently how // to invoke ccache while also invoking our compilers. - if let Some(ref ccache) = builder.config.ccache { - cfg.define("CMAKE_C_COMPILER_LAUNCHER", ccache) - .define("CMAKE_CXX_COMPILER_LAUNCHER", ccache); + if use_compiler_launcher { + if let Some(ref ccache) = builder.config.ccache { + cfg.define("CMAKE_C_COMPILER_LAUNCHER", ccache) + .define("CMAKE_CXX_COMPILER_LAUNCHER", ccache); + } } cfg.define("CMAKE_C_COMPILER", sanitize_cc(cc)) .define("CMAKE_CXX_COMPILER", sanitize_cc(cxx)); @@ -458,7 +465,7 @@ impl Step for Lld { t!(fs::create_dir_all(&out_dir)); let mut cfg = cmake::Config::new(builder.src.join("src/llvm-project/lld")); - configure_cmake(builder, target, &mut cfg); + configure_cmake(builder, target, &mut cfg, true); // This is an awful, awful hack. Discovered when we migrated to using // clang-cl to compile LLVM/LLD it turns out that LLD, when built out of @@ -595,10 +602,7 @@ impl Step for Sanitizers { let _time = util::timeit(&builder); let mut cfg = cmake::Config::new(&compiler_rt_dir); - cfg.target(&self.target); - cfg.host(&builder.config.build); cfg.profile("Release"); - cfg.define("CMAKE_C_COMPILER_TARGET", self.target); cfg.define("COMPILER_RT_BUILD_BUILTINS", "OFF"); cfg.define("COMPILER_RT_BUILD_CRT", "OFF"); @@ -610,6 +614,12 @@ impl Step for Sanitizers { cfg.define("COMPILER_RT_USE_LIBCXX", "OFF"); cfg.define("LLVM_CONFIG_PATH", &llvm_config); + // On Darwin targets the sanitizer runtimes are build as universal binaries. + // Unfortunately sccache currently lacks support to build them successfully. + // Disable compiler launcher on Darwin targets to avoid potential issues. + let use_compiler_launcher = !self.target.contains("apple-darwin"); + configure_cmake(builder, self.target, &mut cfg, use_compiler_launcher); + t!(fs::create_dir_all(&out_dir)); cfg.out_dir(out_dir); From c39b04ea851b821359534b540c0babb97de24122 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Feb 2020 17:19:05 -0800 Subject: [PATCH 14/18] When expecting `BoxFuture` and using `async {}`, suggest `Box::pin` --- .../traits/error_reporting/suggestions.rs | 5 +-- src/librustc_typeck/check/demand.rs | 1 + src/librustc_typeck/check/mod.rs | 44 +++++++++++++++++-- .../expected-boxed-future-isnt-pinned.fixed | 16 +++++++ .../expected-boxed-future-isnt-pinned.rs | 16 +++++++ .../expected-boxed-future-isnt-pinned.stderr | 27 ++++++++++++ 6 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed create mode 100644 src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs create mode 100644 src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr diff --git a/src/librustc/traits/error_reporting/suggestions.rs b/src/librustc/traits/error_reporting/suggestions.rs index 60e55bd7bd9ad..82b73518d09a8 100644 --- a/src/librustc/traits/error_reporting/suggestions.rs +++ b/src/librustc/traits/error_reporting/suggestions.rs @@ -701,10 +701,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }) .collect::>(); // Add the suggestion for the return type. - suggestions.push(( - ret_ty.span, - format!("Box<{}{}>", if has_dyn { "" } else { "dyn " }, snippet), - )); + suggestions.push((ret_ty.span, format!("Box", trait_obj))); err.multipart_suggestion( "return a boxed trait object instead", suggestions, diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 3c2e02fc79b3a..ac5214ca756b2 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -25,6 +25,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.suggest_compatible_variants(err, expr, expected, expr_ty); self.suggest_ref_or_into(err, expr, expected, expr_ty); self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); + self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty); self.suggest_missing_await(err, expr, expected, expr_ty); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index be2052dce3c0b..03c1f97246df9 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5038,10 +5038,46 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); err.note( - "for more on the distinction between the stack and the \ - heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ - https://doc.rust-lang.org/rust-by-example/std/box.html, and \ - https://doc.rust-lang.org/std/boxed/index.html", + "for more on the distinction between the stack and the heap, read \ + https://doc.rust-lang.org/book/ch15-01-box.html, \ + https://doc.rust-lang.org/rust-by-example/std/box.html, and \ + https://doc.rust-lang.org/std/boxed/index.html", + ); + } + } + + /// When encountering an `impl Future` where `BoxFuture` is expected, suggest `Box::pin`. + fn suggest_calling_boxed_future_when_appropriate( + &self, + err: &mut DiagnosticBuilder<'_>, + expr: &hir::Expr<'_>, + expected: Ty<'tcx>, + found: Ty<'tcx>, + ) { + // Handle #68197. + + if self.tcx.hir().is_const_context(expr.hir_id) { + // Do not suggest `Box::new` in const context. + return; + } + let pin_did = self.tcx.lang_items().pin_type(); + match expected.kind { + ty::Adt(def, _) if Some(def.did) != pin_did => return, + // This guards the `unwrap` and `mk_box` below. + _ if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() => return, + _ => {} + } + let boxed_found = self.tcx.mk_box(found); + let new_found = self.tcx.mk_lang_item(boxed_found, lang_items::PinTypeLangItem).unwrap(); + if let (true, Ok(snippet)) = ( + self.can_coerce(new_found, expected), + self.sess().source_map().span_to_snippet(expr.span), + ) { + err.span_suggestion( + expr.span, + "you need to pin and box this expression", + format!("Box::pin({})", snippet), + Applicability::MachineApplicable, ); } } diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed new file mode 100644 index 0000000000000..bddfd3ac9ccf0 --- /dev/null +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed @@ -0,0 +1,16 @@ +// edition:2018 +// run-rustfix +#![allow(dead_code)] +use std::future::Future; +use std::pin::Pin; + +type BoxFuture<'a, T> = Pin + Send + 'a>>; +// ^^^^^^^^^ This would come from the `futures` crate in real code. + +fn foo() -> BoxFuture<'static, i32> { + Box::pin(async { //~ ERROR mismatched types + 42 + }) +} + +fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs new file mode 100644 index 0000000000000..51818d6ae8f69 --- /dev/null +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -0,0 +1,16 @@ +// edition:2018 +// run-rustfix +#![allow(dead_code)] +use std::future::Future; +use std::pin::Pin; + +type BoxFuture<'a, T> = Pin + Send + 'a>>; +// ^^^^^^^^^ This would come from the `futures` crate in real code. + +fn foo() -> BoxFuture<'static, i32> { + async { //~ ERROR mismatched types + 42 + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr new file mode 100644 index 0000000000000..5e6f5c13b7a60 --- /dev/null +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -0,0 +1,27 @@ +error[E0308]: mismatched types + --> $DIR/expected-boxed-future-isnt-pinned.rs:11:5 + | +LL | fn foo() -> BoxFuture<'static, i32> { + | ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type +LL | / async { +LL | | 42 +LL | | } + | |_____^ expected struct `std::pin::Pin`, found opaque type + | + ::: $SRC_DIR/libstd/future.rs:LL:COL + | +LL | pub fn from_generator>(x: T) -> impl Future { + | ------------------------------- the found opaque type + | + = note: expected struct `std::pin::Pin + std::marker::Send + 'static)>>` + found opaque type `impl std::future::Future` +help: you need to pin and box this expression + | +LL | Box::pin(async { +LL | 42 +LL | }) + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From a852fb74131af7473bafb03d0f3994a0e9f597d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 11 Feb 2020 17:35:04 -0800 Subject: [PATCH 15/18] Remove std lib `Span` from expected boxed future test --- .../expected-boxed-future-isnt-pinned.fixed | 7 ++-- .../expected-boxed-future-isnt-pinned.rs | 7 ++-- .../expected-boxed-future-isnt-pinned.stderr | 33 ++++++++----------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed index bddfd3ac9ccf0..9c68de7bacec6 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed @@ -7,10 +7,9 @@ use std::pin::Pin; type BoxFuture<'a, T> = Pin + Send + 'a>>; // ^^^^^^^^^ This would come from the `futures` crate in real code. -fn foo() -> BoxFuture<'static, i32> { - Box::pin(async { //~ ERROR mismatched types - 42 - }) +fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + // We could instead use an `async` block, but this way we have no std spans. + Box::pin(x) //~ ERROR mismatched types } fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs index 51818d6ae8f69..0b5200fc25ca0 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -7,10 +7,9 @@ use std::pin::Pin; type BoxFuture<'a, T> = Pin + Send + 'a>>; // ^^^^^^^^^ This would come from the `futures` crate in real code. -fn foo() -> BoxFuture<'static, i32> { - async { //~ ERROR mismatched types - 42 - } +fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + // We could instead use an `async` block, but this way we have no std spans. + x //~ ERROR mismatched types } fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr index 5e6f5c13b7a60..5e54fc246a20f 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -1,26 +1,19 @@ error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:11:5 + --> $DIR/expected-boxed-future-isnt-pinned.rs:12:5 | -LL | fn foo() -> BoxFuture<'static, i32> { - | ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type -LL | / async { -LL | | 42 -LL | | } - | |_____^ expected struct `std::pin::Pin`, found opaque type - | - ::: $SRC_DIR/libstd/future.rs:LL:COL - | -LL | pub fn from_generator>(x: T) -> impl Future { - | ------------------------------- the found opaque type - | - = note: expected struct `std::pin::Pin + std::marker::Send + 'static)>>` - found opaque type `impl std::future::Future` -help: you need to pin and box this expression - | -LL | Box::pin(async { -LL | 42 -LL | }) +LL | fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + | - this type parameter ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type +LL | // We could instead use an `async` block, but this way we have no std spans. +LL | x + | ^ + | | + | expected struct `std::pin::Pin`, found type parameter `F` + | help: you need to pin and box this expression: `Box::pin(x)` | + = note: expected struct `std::pin::Pin + std::marker::Send + 'static)>>` + found type parameter `F` + = help: type parameters must be constrained to match other types + = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters error: aborting due to previous error From 80cdb0af7dd27471e1e4a4362e2473a9331a5fdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 12 Feb 2020 11:11:55 -0800 Subject: [PATCH 16/18] Account for `Box::new(impl Future)` and emit help `use Box::pin` --- src/librustc_typeck/check/demand.rs | 4 +- src/librustc_typeck/check/mod.rs | 30 +++++++---- .../expected-boxed-future-isnt-pinned.fixed | 15 ------ .../expected-boxed-future-isnt-pinned.rs | 13 ++++- .../expected-boxed-future-isnt-pinned.stderr | 52 +++++++++++++++++-- 5 files changed, 84 insertions(+), 30 deletions(-) delete mode 100644 src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index ac5214ca756b2..4a98095ec89c6 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -24,8 +24,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.annotate_expected_due_to_let_ty(err, expr); self.suggest_compatible_variants(err, expr, expected, expr_ty); self.suggest_ref_or_into(err, expr, expected, expr_ty); + if self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty) { + return; + } self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); - self.suggest_calling_boxed_future_when_appropriate(err, expr, expected, expr_ty); self.suggest_missing_await(err, expr, expected, expr_ty); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 03c1f97246df9..6d37526df2c22 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5053,18 +5053,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, expected: Ty<'tcx>, found: Ty<'tcx>, - ) { + ) -> bool { // Handle #68197. if self.tcx.hir().is_const_context(expr.hir_id) { // Do not suggest `Box::new` in const context. - return; + return false; } let pin_did = self.tcx.lang_items().pin_type(); match expected.kind { - ty::Adt(def, _) if Some(def.did) != pin_did => return, + ty::Adt(def, _) if Some(def.did) != pin_did => return false, // This guards the `unwrap` and `mk_box` below. - _ if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() => return, + _ if pin_did.is_none() || self.tcx.lang_items().owned_box().is_none() => return false, _ => {} } let boxed_found = self.tcx.mk_box(found); @@ -5073,12 +5073,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.can_coerce(new_found, expected), self.sess().source_map().span_to_snippet(expr.span), ) { - err.span_suggestion( - expr.span, - "you need to pin and box this expression", - format!("Box::pin({})", snippet), - Applicability::MachineApplicable, - ); + match found.kind { + ty::Adt(def, _) if def.is_box() => { + err.help("use `Box::pin`"); + } + _ => { + err.span_suggestion( + expr.span, + "you need to pin and box this expression", + format!("Box::pin({})", snippet), + Applicability::MachineApplicable, + ); + } + } + true + } else { + false } } diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed deleted file mode 100644 index 9c68de7bacec6..0000000000000 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.fixed +++ /dev/null @@ -1,15 +0,0 @@ -// edition:2018 -// run-rustfix -#![allow(dead_code)] -use std::future::Future; -use std::pin::Pin; - -type BoxFuture<'a, T> = Pin + Send + 'a>>; -// ^^^^^^^^^ This would come from the `futures` crate in real code. - -fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - // We could instead use an `async` block, but this way we have no std spans. - Box::pin(x) //~ ERROR mismatched types -} - -fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs index 0b5200fc25ca0..fd40776539035 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -1,5 +1,4 @@ // edition:2018 -// run-rustfix #![allow(dead_code)] use std::future::Future; use std::pin::Pin; @@ -11,5 +10,17 @@ fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> // We could instead use an `async` block, but this way we have no std spans. x //~ ERROR mismatched types } +fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + Box::new(x) //~ ERROR mismatched types + //~^ HELP use `Box::pin` +} +fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + Pin::new(x) //~ ERROR mismatched types + //~^ ERROR the trait bound +} +fn qux + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + Pin::new(Box::new(x)) //~ ERROR mismatched types + //~^ ERROR the trait bound +} fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr index 5e54fc246a20f..cf5ef1362db3f 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:12:5 + --> $DIR/expected-boxed-future-isnt-pinned.rs:11:5 | LL | fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> { | - this type parameter ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type @@ -15,6 +15,52 @@ LL | x = help: type parameters must be constrained to match other types = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/expected-boxed-future-isnt-pinned.rs:14:5 + | +LL | fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + | ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type +LL | Box::new(x) + | ^^^^^^^^^^^ expected struct `std::pin::Pin`, found struct `std::boxed::Box` + | + = note: expected struct `std::pin::Pin + std::marker::Send + 'static)>>` + found struct `std::boxed::Box` + = help: use `Box::pin` + +error[E0308]: mismatched types + --> $DIR/expected-boxed-future-isnt-pinned.rs:18:14 + | +LL | fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { + | - this type parameter +LL | Pin::new(x) + | ^ + | | + | expected struct `std::boxed::Box`, found type parameter `F` + | help: store this in the heap by calling `Box::new`: `Box::new(x)` + | + = note: expected struct `std::boxed::Box + std::marker::Send>` + found type parameter `F` + = help: type parameters must be constrained to match other types + = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters + = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html + +error[E0277]: the trait bound `dyn std::future::Future + std::marker::Send: std::marker::Unpin` is not satisfied + --> $DIR/expected-boxed-future-isnt-pinned.rs:18:5 + | +LL | Pin::new(x) + | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` + | + = note: required by `std::pin::Pin::

::new` + +error[E0277]: the trait bound `dyn std::future::Future + std::marker::Send: std::marker::Unpin` is not satisfied + --> $DIR/expected-boxed-future-isnt-pinned.rs:22:5 + | +LL | Pin::new(Box::new(x)) + | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` + | + = note: required by `std::pin::Pin::

::new` + +error: aborting due to 5 previous errors -For more information about this error, try `rustc --explain E0308`. +Some errors have detailed explanations: E0277, E0308. +For more information about an error, try `rustc --explain E0277`. From c376fc001772200e2de8d7a610a5b67dcf642432 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 12 Feb 2020 11:51:07 -0800 Subject: [PATCH 17/18] Account for `Pin::new(_)` and `Pin::new(Box::new(_))` when `Box::pin(_)` would be applicable --- src/libcore/marker.rs | 7 +++++++ src/test/ui/generator/static-not-unpin.rs | 2 +- src/test/ui/generator/static-not-unpin.stderr | 2 +- .../expected-boxed-future-isnt-pinned.rs | 9 +++++---- .../expected-boxed-future-isnt-pinned.stderr | 14 ++++++++------ 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index b4b595f330e22..5c35041249f36 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -727,6 +727,13 @@ unsafe impl Freeze for &mut T {} /// [`Pin

`]: ../pin/struct.Pin.html /// [`pin module`]: ../../std/pin/index.html #[stable(feature = "pin", since = "1.33.0")] +#[rustc_on_unimplemented( + on( + _Self = "dyn std::future::Future + std::marker::Send", + note = "consider using `Box::pin`", + ), + message = "`{Self}` cannot be unpinned" +)] #[lang = "unpin"] pub auto trait Unpin {} diff --git a/src/test/ui/generator/static-not-unpin.rs b/src/test/ui/generator/static-not-unpin.rs index b271e982fb420..cfcb94737be6f 100644 --- a/src/test/ui/generator/static-not-unpin.rs +++ b/src/test/ui/generator/static-not-unpin.rs @@ -11,5 +11,5 @@ fn main() { let mut generator = static || { yield; }; - assert_unpin(generator); //~ ERROR std::marker::Unpin` is not satisfied + assert_unpin(generator); //~ ERROR E0277 } diff --git a/src/test/ui/generator/static-not-unpin.stderr b/src/test/ui/generator/static-not-unpin.stderr index f2b1078e2b532..6512d67319b0b 100644 --- a/src/test/ui/generator/static-not-unpin.stderr +++ b/src/test/ui/generator/static-not-unpin.stderr @@ -1,4 +1,4 @@ -error[E0277]: the trait bound `[static generator@$DIR/static-not-unpin.rs:11:25: 13:6 _]: std::marker::Unpin` is not satisfied +error[E0277]: `[static generator@$DIR/static-not-unpin.rs:11:25: 13:6 _]` cannot be unpinned --> $DIR/static-not-unpin.rs:14:18 | LL | fn assert_unpin(_: T) { diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs index fd40776539035..eda579f7fb49e 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -10,17 +10,18 @@ fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> // We could instead use an `async` block, but this way we have no std spans. x //~ ERROR mismatched types } + fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { Box::new(x) //~ ERROR mismatched types - //~^ HELP use `Box::pin` } + fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { Pin::new(x) //~ ERROR mismatched types - //~^ ERROR the trait bound + //~^ ERROR E0277 } + fn qux + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - Pin::new(Box::new(x)) //~ ERROR mismatched types - //~^ ERROR the trait bound + Pin::new(Box::new(x)) //~ ERROR E0277 } fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr index cf5ef1362db3f..783b118d2f9a2 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -16,7 +16,7 @@ LL | x = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:14:5 + --> $DIR/expected-boxed-future-isnt-pinned.rs:15:5 | LL | fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { | ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type @@ -28,7 +28,7 @@ LL | Box::new(x) = help: use `Box::pin` error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:18:14 + --> $DIR/expected-boxed-future-isnt-pinned.rs:19:14 | LL | fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { | - this type parameter @@ -44,20 +44,22 @@ LL | Pin::new(x) = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html -error[E0277]: the trait bound `dyn std::future::Future + std::marker::Send: std::marker::Unpin` is not satisfied - --> $DIR/expected-boxed-future-isnt-pinned.rs:18:5 +error[E0277]: `dyn std::future::Future + std::marker::Send` cannot be unpinned + --> $DIR/expected-boxed-future-isnt-pinned.rs:19:5 | LL | Pin::new(x) | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` | + = note: consider using `Box::pin` = note: required by `std::pin::Pin::

::new` -error[E0277]: the trait bound `dyn std::future::Future + std::marker::Send: std::marker::Unpin` is not satisfied - --> $DIR/expected-boxed-future-isnt-pinned.rs:22:5 +error[E0277]: `dyn std::future::Future + std::marker::Send` cannot be unpinned + --> $DIR/expected-boxed-future-isnt-pinned.rs:24:5 | LL | Pin::new(Box::new(x)) | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` | + = note: consider using `Box::pin` = note: required by `std::pin::Pin::

::new` error: aborting due to 5 previous errors From 248f5a4046ab5a90189f37c305c759b7cde8acb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 12 Feb 2020 16:50:28 -0800 Subject: [PATCH 18/18] Add trait `Self` filtering to `rustc_on_unimplemented` --- src/libcore/marker.rs | 5 +- .../error_reporting/on_unimplemented.rs | 10 ++++ .../expected-boxed-future-isnt-pinned.rs | 26 +++++----- .../expected-boxed-future-isnt-pinned.stderr | 52 +------------------ 4 files changed, 27 insertions(+), 66 deletions(-) diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 5c35041249f36..2800f11cc01b1 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -728,10 +728,7 @@ unsafe impl Freeze for &mut T {} /// [`pin module`]: ../../std/pin/index.html #[stable(feature = "pin", since = "1.33.0")] #[rustc_on_unimplemented( - on( - _Self = "dyn std::future::Future + std::marker::Send", - note = "consider using `Box::pin`", - ), + on(_Self = "std::future::Future", note = "consider using `Box::pin`",), message = "`{Self}` cannot be unpinned" )] #[lang = "unpin"] diff --git a/src/librustc/traits/error_reporting/on_unimplemented.rs b/src/librustc/traits/error_reporting/on_unimplemented.rs index 2ba12baaf6d6e..ab2d74b1c8deb 100644 --- a/src/librustc/traits/error_reporting/on_unimplemented.rs +++ b/src/librustc/traits/error_reporting/on_unimplemented.rs @@ -201,6 +201,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } } + if let ty::Dynamic(traits, _) = self_ty.kind { + for t in *traits.skip_binder() { + match t { + ty::ExistentialPredicate::Trait(trait_ref) => { + flags.push((sym::_Self, Some(self.tcx.def_path_str(trait_ref.def_id)))) + } + _ => {} + } + } + } if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, trait_ref.def_id, def_id) diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs index eda579f7fb49e..0a1686eac9d34 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.rs @@ -11,17 +11,19 @@ fn foo + Send + 'static>(x: F) -> BoxFuture<'static, i32> x //~ ERROR mismatched types } -fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - Box::new(x) //~ ERROR mismatched types -} - -fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - Pin::new(x) //~ ERROR mismatched types - //~^ ERROR E0277 -} - -fn qux + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - Pin::new(Box::new(x)) //~ ERROR E0277 -} +// FIXME: uncomment these once this commit is in Beta and we can rely on `rustc_on_unimplemented` +// having filtering for `Self` being a trait. +// +// fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { +// Box::new(x) +// } +// +// fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { +// Pin::new(x) +// } +// +// fn qux + Send + 'static>(x: F) -> BoxFuture<'static, i32> { +// Pin::new(Box::new(x)) +// } fn main() {} diff --git a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr index 783b118d2f9a2..48d941283b62d 100644 --- a/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr +++ b/src/test/ui/suggestions/expected-boxed-future-isnt-pinned.stderr @@ -15,54 +15,6 @@ LL | x = help: type parameters must be constrained to match other types = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters -error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:15:5 - | -LL | fn bar + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - | ----------------------- expected `std::pin::Pin + std::marker::Send + 'static)>>` because of return type -LL | Box::new(x) - | ^^^^^^^^^^^ expected struct `std::pin::Pin`, found struct `std::boxed::Box` - | - = note: expected struct `std::pin::Pin + std::marker::Send + 'static)>>` - found struct `std::boxed::Box` - = help: use `Box::pin` - -error[E0308]: mismatched types - --> $DIR/expected-boxed-future-isnt-pinned.rs:19:14 - | -LL | fn baz + Send + 'static>(x: F) -> BoxFuture<'static, i32> { - | - this type parameter -LL | Pin::new(x) - | ^ - | | - | expected struct `std::boxed::Box`, found type parameter `F` - | help: store this in the heap by calling `Box::new`: `Box::new(x)` - | - = note: expected struct `std::boxed::Box + std::marker::Send>` - found type parameter `F` - = help: type parameters must be constrained to match other types - = note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters - = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html - -error[E0277]: `dyn std::future::Future + std::marker::Send` cannot be unpinned - --> $DIR/expected-boxed-future-isnt-pinned.rs:19:5 - | -LL | Pin::new(x) - | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` - | - = note: consider using `Box::pin` - = note: required by `std::pin::Pin::

::new` - -error[E0277]: `dyn std::future::Future + std::marker::Send` cannot be unpinned - --> $DIR/expected-boxed-future-isnt-pinned.rs:24:5 - | -LL | Pin::new(Box::new(x)) - | ^^^^^^^^ the trait `std::marker::Unpin` is not implemented for `dyn std::future::Future + std::marker::Send` - | - = note: consider using `Box::pin` - = note: required by `std::pin::Pin::

::new` - -error: aborting due to 5 previous errors +error: aborting due to previous error -Some errors have detailed explanations: E0277, E0308. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0308`.