From d6f385fe75979c0d919367bcd3e1c65df793421d Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 6 Sep 2022 10:49:09 -0700 Subject: [PATCH 01/15] rustdoc mobile: move notable traits to return type These were originally on the left, but were moved to the return type in c90fb7185a5febb00b7f8ccb49abceacd41bad6e. The CSS rule for mobile did not get updated at the time, so updating it now. --- src/librustdoc/html/static/css/rustdoc.css | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 83fe14550cc21..88a412314e058 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1973,12 +1973,6 @@ in storage.js plus the media query with (min-width: 701px) display: none !important; } - .notable-traits { - position: absolute; - left: -22px; - top: 24px; - } - #titles > button > div.count { float: left; width: 100%; From fb42dae9870c8e1085e9e524363123741a49a255 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Sep 2022 17:38:44 +0200 Subject: [PATCH 02/15] Simplify CSS parser to check themes --- src/librustdoc/config.rs | 16 +- src/librustdoc/theme.rs | 369 +++++++++++++++++---------------------- 2 files changed, 178 insertions(+), 207 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 8a8cc272e8195..932533db05c14 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -412,7 +412,13 @@ impl Options { let to_check = matches.opt_strs("check-theme"); if !to_check.is_empty() { - let paths = theme::load_css_paths(static_files::themes::LIGHT.as_bytes()); + let paths = match theme::load_css_paths(static_files::themes::LIGHT) { + Ok(p) => p, + Err(e) => { + diag.struct_err(&e.to_string()).emit(); + return Err(1); + } + }; let mut errors = 0; println!("rustdoc: [check-theme] Starting tests! (Ignoring all other arguments)"); @@ -547,7 +553,13 @@ impl Options { let mut themes = Vec::new(); if matches.opt_present("theme") { - let paths = theme::load_css_paths(static_files::themes::LIGHT.as_bytes()); + let paths = match theme::load_css_paths(static_files::themes::LIGHT) { + Ok(p) => p, + Err(e) => { + diag.struct_err(&e.to_string()).emit(); + return Err(1); + } + }; for (theme_file, theme_s) in matches.opt_strs("theme").iter().map(|s| (PathBuf::from(&s), s.to_owned())) diff --git a/src/librustdoc/theme.rs b/src/librustdoc/theme.rs index 0118d7dd20722..77f8359bd42ca 100644 --- a/src/librustdoc/theme.rs +++ b/src/librustdoc/theme.rs @@ -1,271 +1,230 @@ -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::FxHashMap; +use std::collections::hash_map::Entry; use std::fs; -use std::hash::{Hash, Hasher}; +use std::iter::Peekable; use std::path::Path; +use std::str::Chars; use rustc_errors::Handler; #[cfg(test)] mod tests; -#[derive(Debug, Clone, Eq)] +#[derive(Debug)] pub(crate) struct CssPath { - pub(crate) name: String, - pub(crate) children: FxHashSet, + pub(crate) rules: FxHashMap, + pub(crate) children: FxHashMap, } -// This PartialEq implementation IS NOT COMMUTATIVE!!! -// -// The order is very important: the second object must have all first's rules. -// However, the first is not required to have all of the second's rules. -impl PartialEq for CssPath { - fn eq(&self, other: &CssPath) -> bool { - if self.name != other.name { - false - } else { - for child in &self.children { - if !other.children.iter().any(|c| child == c) { - return false; - } - } - true - } - } -} +/// When encountering a `"` or a `'`, returns the whole string, including the quote characters. +fn get_string(iter: &mut Peekable>, string_start: char) -> String { + let mut s = String::with_capacity(2); -impl Hash for CssPath { - fn hash(&self, state: &mut H) { - self.name.hash(state); - for x in &self.children { - x.hash(state); + s.push(string_start); + while let Some(c) = iter.next() { + s.push(c); + if c == '\\' { + iter.next(); + } else if c == string_start { + break; } } + s } -impl CssPath { - fn new(name: String) -> CssPath { - CssPath { name, children: FxHashSet::default() } +/// Skips a `/*` comment. +fn skip_comment(iter: &mut Peekable>) { + while let Some(c) = iter.next() { + if c == '*' && iter.next() == Some('/') { + break; + } } } -/// All variants contain the position they occur. -#[derive(Debug, Clone, Copy)] -enum Events { - StartLineComment(usize), - StartComment(usize), - EndComment(usize), - InBlock(usize), - OutBlock(usize), -} - -impl Events { - fn get_pos(&self) -> usize { - match *self { - Events::StartLineComment(p) - | Events::StartComment(p) - | Events::EndComment(p) - | Events::InBlock(p) - | Events::OutBlock(p) => p, +/// Skips a line comment (`//`). +fn skip_line_comment(iter: &mut Peekable>) { + while let Some(c) = iter.next() { + if c == '\n' { + break; } } - - fn is_comment(&self) -> bool { - matches!( - self, - Events::StartLineComment(_) | Events::StartComment(_) | Events::EndComment(_) - ) - } } -fn previous_is_line_comment(events: &[Events]) -> bool { - matches!(events.last(), Some(&Events::StartLineComment(_))) -} - -fn is_line_comment(pos: usize, v: &[u8], events: &[Events]) -> bool { - if let Some(&Events::StartComment(_)) = events.last() { - return false; +fn handle_common_chars(c: char, buffer: &mut String, iter: &mut Peekable>) { + match c { + '"' | '\'' => buffer.push_str(&get_string(iter, c)), + '/' if iter.peek() == Some(&'*') => skip_comment(iter), + '/' if iter.peek() == Some(&'/') => skip_line_comment(iter), + _ => buffer.push(c), } - v[pos + 1] == b'/' } -fn load_css_events(v: &[u8]) -> Vec { - let mut pos = 0; - let mut events = Vec::with_capacity(100); - - while pos + 1 < v.len() { - match v[pos] { - b'/' if v[pos + 1] == b'*' => { - events.push(Events::StartComment(pos)); - pos += 1; - } - b'/' if is_line_comment(pos, v, &events) => { - events.push(Events::StartLineComment(pos)); - pos += 1; - } - b'\n' if previous_is_line_comment(&events) => { - events.push(Events::EndComment(pos)); - } - b'*' if v[pos + 1] == b'/' => { - events.push(Events::EndComment(pos + 2)); - pos += 1; - } - b'{' if !previous_is_line_comment(&events) => { - if let Some(&Events::StartComment(_)) = events.last() { - pos += 1; - continue; - } - events.push(Events::InBlock(pos + 1)); - } - b'}' if !previous_is_line_comment(&events) => { - if let Some(&Events::StartComment(_)) = events.last() { - pos += 1; - continue; - } - events.push(Events::OutBlock(pos + 1)); - } - _ => {} +/// Returns a CSS property name. Ends when encountering a `:` character. +/// +/// If the `:` character isn't found, returns `None`. +/// +/// If a `{` character is encountered, returns an error. +fn parse_property_name(iter: &mut Peekable>) -> Result, String> { + let mut content = String::new(); + + while let Some(c) = iter.next() { + match c { + ':' => return Ok(Some(content.trim().to_owned())), + '{' => return Err("Unexpected `{` in a `{}` block".to_owned()), + '}' => break, + _ => handle_common_chars(c, &mut content, iter), } - pos += 1; } - events -} - -fn get_useful_next(events: &[Events], pos: &mut usize) -> Option { - while *pos < events.len() { - if !events[*pos].is_comment() { - return Some(events[*pos]); + Ok(None) +} + +/// Try to get the value of a CSS property (the `#fff` in `color: #fff`). It'll stop when it +/// encounters a `{` or a `;` character. +/// +/// It returns the value string and a boolean set to `true` if the value is ended with a `}` because +/// it means that the parent block is done and that we should notify the parent caller. +fn parse_property_value(iter: &mut Peekable>) -> (String, bool) { + let mut value = String::new(); + let mut out_block = false; + + while let Some(c) = iter.next() { + match c { + ';' => break, + '}' => { + out_block = true; + break; + } + _ => handle_common_chars(c, &mut value, iter), } - *pos += 1; } - None + (value.trim().to_owned(), out_block) } -fn get_previous_positions(events: &[Events], mut pos: usize) -> Vec { - let mut ret = Vec::with_capacity(3); +/// This is used to parse inside a CSS `{}` block. If we encounter a new `{` inside it, we consider +/// it as a new block and therefore recurse into `parse_rules`. +fn parse_rules( + content: &str, + selector: String, + iter: &mut Peekable>, + paths: &mut FxHashMap, +) -> Result<(), String> { + let mut rules = FxHashMap::default(); + let mut children = FxHashMap::default(); - ret.push(events[pos].get_pos()); - if pos > 0 { - pos -= 1; - } loop { - if pos < 1 || !events[pos].is_comment() { - let x = events[pos].get_pos(); - if *ret.last().unwrap() != x { - ret.push(x); - } else { - ret.push(0); + // If the parent isn't a "normal" CSS selector, we only expect sub-selectors and not CSS + // properties. + if selector.starts_with('@') { + parse_selectors(content, iter, &mut children)?; + break; + } + let rule = match parse_property_name(iter)? { + Some(r) => { + if r.is_empty() { + return Err(format!("Found empty rule in selector `{selector}`")); + } + r } + None => break, + }; + let (value, out_block) = parse_property_value(iter); + if value.is_empty() { + return Err(format!("Found empty value for rule `{rule}` in selector `{selector}`")); + } + match rules.entry(rule) { + Entry::Occupied(mut o) => { + eprintln!("Duplicated rule `{}` in CSS selector `{selector}`", o.key()); + *o.get_mut() = value; + } + Entry::Vacant(v) => { + v.insert(value); + } + } + if out_block { break; } - ret.push(events[pos].get_pos()); - pos -= 1; } - if ret.len() & 1 != 0 && events[pos].is_comment() { - ret.push(0); - } - ret.iter().rev().cloned().collect() -} -fn build_rule(v: &[u8], positions: &[usize]) -> String { - minifier::css::minify( - &positions - .chunks(2) - .map(|x| ::std::str::from_utf8(&v[x[0]..x[1]]).unwrap_or("")) - .collect::() - .trim() - .chars() - .filter_map(|c| match c { - '\n' | '\t' => Some(' '), - '/' | '{' | '}' => None, - c => Some(c), - }) - .collect::() - .split(' ') - .filter(|s| !s.is_empty()) - .intersperse(" ") - .collect::(), - ) - .map(|css| css.to_string()) - .unwrap_or_else(|_| String::new()) -} - -fn inner(v: &[u8], events: &[Events], pos: &mut usize) -> FxHashSet { - let mut paths = Vec::with_capacity(50); - - while *pos < events.len() { - if let Some(Events::OutBlock(_)) = get_useful_next(events, pos) { - *pos += 1; - break; + match paths.entry(selector) { + Entry::Occupied(mut o) => { + eprintln!("Duplicated CSS selector: `{}`", o.key()); + let v = o.get_mut(); + for (key, value) in rules.into_iter() { + v.rules.insert(key, value); + } + for (sel, child) in children.into_iter() { + v.children.insert(sel, child); + } } - if let Some(Events::InBlock(_)) = get_useful_next(events, pos) { - paths.push(CssPath::new(build_rule(v, &get_previous_positions(events, *pos)))); - *pos += 1; + Entry::Vacant(v) => { + v.insert(CssPath { rules, children }); } - while let Some(Events::InBlock(_)) = get_useful_next(events, pos) { - if let Some(ref mut path) = paths.last_mut() { - for entry in inner(v, events, pos).iter() { - path.children.insert(entry.clone()); - } + } + Ok(()) +} + +pub(crate) fn parse_selectors( + content: &str, + iter: &mut Peekable>, + paths: &mut FxHashMap, +) -> Result<(), String> { + let mut selector = String::new(); + + while let Some(c) = iter.next() { + match c { + '{' => { + let s = minifier::css::minify(selector.trim()).map(|s| s.to_string())?; + parse_rules(content, s, iter, paths)?; + selector.clear(); } - } - if let Some(Events::OutBlock(_)) = get_useful_next(events, pos) { - *pos += 1; + '}' => break, + ';' => selector.clear(), // We don't handle inline selectors like `@import`. + _ => handle_common_chars(c, &mut selector, iter), } } - paths.iter().cloned().collect() + Ok(()) } -pub(crate) fn load_css_paths(v: &[u8]) -> CssPath { - let events = load_css_events(v); - let mut pos = 0; +/// The entry point to parse the CSS rules. Every time we encounter a `{`, we then parse the rules +/// inside it. +pub(crate) fn load_css_paths(content: &str) -> Result, String> { + let mut iter = content.chars().peekable(); + let mut paths = FxHashMap::default(); - let mut parent = CssPath::new("parent".to_owned()); - parent.children = inner(v, &events, &mut pos); - parent + parse_selectors(content, &mut iter, &mut paths)?; + Ok(paths) } -pub(crate) fn get_differences(against: &CssPath, other: &CssPath, v: &mut Vec) { - if against.name == other.name { - for child in &against.children { - let mut found = false; - let mut found_working = false; - let mut tmp = Vec::new(); - - for other_child in &other.children { - if child.name == other_child.name { - if child != other_child { - get_differences(child, other_child, &mut tmp); - } else { - found_working = true; - } - found = true; - break; - } - } - if !found { - v.push(format!(" Missing \"{}\" rule", child.name)); - } else if !found_working { - v.extend(tmp.iter().cloned()); - } +pub(crate) fn get_differences( + origin: &FxHashMap, + against: &FxHashMap, + v: &mut Vec, +) { + for (selector, entry) in origin.iter() { + match against.get(selector) { + Some(a) => get_differences(&a.children, &entry.children, v), + None => v.push(format!(" Missing rule `{}`", selector)), } } } pub(crate) fn test_theme_against>( f: &P, - against: &CssPath, + origin: &FxHashMap, diag: &Handler, ) -> (bool, Vec) { - let data = match fs::read(f) { + let against = match fs::read_to_string(f) + .map_err(|e| e.to_string()) + .and_then(|data| load_css_paths(&data)) + { Ok(c) => c, Err(e) => { - diag.struct_err(&e.to_string()).emit(); + diag.struct_err(&e).emit(); return (false, vec![]); } }; - let paths = load_css_paths(&data); let mut ret = vec![]; - get_differences(against, &paths, &mut ret); + get_differences(origin, &against, &mut ret); (true, ret) } From 0b037c17b867cc8d47c0391b3d681fbf5a66ce80 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Sep 2022 17:39:32 +0200 Subject: [PATCH 03/15] Update theme check tests --- src/librustdoc/theme/tests.rs | 60 +++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/theme/tests.rs b/src/librustdoc/theme/tests.rs index ae8f43c6d55ba..54226c6cf3b15 100644 --- a/src/librustdoc/theme/tests.rs +++ b/src/librustdoc/theme/tests.rs @@ -44,11 +44,7 @@ rule j end {} "#; let mut ret = Vec::new(); - get_differences( - &load_css_paths(against.as_bytes()), - &load_css_paths(text.as_bytes()), - &mut ret, - ); + get_differences(&load_css_paths(against).unwrap(), &load_css_paths(text).unwrap(), &mut ret); assert!(ret.is_empty()); } @@ -61,46 +57,45 @@ a c // sdf d {} "#; - let paths = load_css_paths(text.as_bytes()); - assert!(paths.children.contains(&CssPath::new("a b c d".to_owned()))); + let paths = load_css_paths(text).unwrap(); + assert!(paths.contains_key(&"a b c d".to_owned())); } #[test] fn test_comparison() { let x = r#" -a { - b { - c {} - } +@a { + b {} } "#; let y = r#" -a { +@a { b {} + c {} } "#; - let against = load_css_paths(y.as_bytes()); - let other = load_css_paths(x.as_bytes()); + let against = load_css_paths(y).unwrap(); + let other = load_css_paths(x).unwrap(); let mut ret = Vec::new(); get_differences(&against, &other, &mut ret); assert!(ret.is_empty()); get_differences(&other, &against, &mut ret); - assert_eq!(ret, vec![" Missing \"c\" rule".to_owned()]); + assert_eq!(ret, vec![" Missing rule `c`".to_owned()]); } #[test] fn check_empty_css() { - let events = load_css_events(&[]); - assert_eq!(events.len(), 0); + let paths = load_css_paths("").unwrap(); + assert_eq!(paths.len(), 0); } #[test] fn check_invalid_css() { - let events = load_css_events(b"*"); - assert_eq!(events.len(), 0); + let paths = load_css_paths("*").unwrap(); + assert_eq!(paths.len(), 0); } #[test] @@ -108,10 +103,33 @@ fn test_with_minification() { let text = include_str!("../html/static/css/themes/dark.css"); let minified = minifier::css::minify(&text).expect("CSS minification failed").to_string(); - let against = load_css_paths(text.as_bytes()); - let other = load_css_paths(minified.as_bytes()); + let against = load_css_paths(text).unwrap(); + let other = load_css_paths(&minified).unwrap(); let mut ret = Vec::new(); get_differences(&against, &other, &mut ret); assert!(ret.is_empty()); } + +#[test] +fn test_media() { + let text = r#" +@media (min-width: 701px) { + a:hover { + color: #fff; + } + + b { + x: y; + } +} +"#; + + let paths = load_css_paths(text).unwrap(); + eprintln!("{:?}", paths); + let p = paths.get("@media (min-width:701px)"); + assert!(p.is_some()); + let p = p.unwrap(); + assert!(p.children.get("a:hover").is_some()); + assert!(p.children.get("b").is_some()); +} From e7d8ad62db3af48ce8fb3844fd44be6251e1cc15 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Sep 2022 18:36:48 +0200 Subject: [PATCH 04/15] Add check for missing CSS variables --- src/librustdoc/theme.rs | 14 ++++++++-- src/librustdoc/theme/tests.rs | 52 +++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/theme.rs b/src/librustdoc/theme.rs index 77f8359bd42ca..87cfe78e10cbd 100644 --- a/src/librustdoc/theme.rs +++ b/src/librustdoc/theme.rs @@ -202,8 +202,18 @@ pub(crate) fn get_differences( ) { for (selector, entry) in origin.iter() { match against.get(selector) { - Some(a) => get_differences(&a.children, &entry.children, v), - None => v.push(format!(" Missing rule `{}`", selector)), + Some(a) => { + get_differences(&entry.children, &a.children, v); + if selector == ":root" { + // We need to check that all variables have been set. + for rule in entry.rules.keys() { + if !a.rules.contains_key(rule) { + v.push(format!(" Missing CSS variable `{rule}` in `:root`")); + } + } + } + } + None => v.push(format!(" Missing rule `{selector}`")), } } } diff --git a/src/librustdoc/theme/tests.rs b/src/librustdoc/theme/tests.rs index 54226c6cf3b15..ce1d77d98d7ad 100644 --- a/src/librustdoc/theme/tests.rs +++ b/src/librustdoc/theme/tests.rs @@ -63,26 +63,26 @@ d {} #[test] fn test_comparison() { - let x = r#" + let origin = r#" @a { b {} + c {} } "#; - let y = r#" + let against = r#" @a { b {} - c {} } "#; - let against = load_css_paths(y).unwrap(); - let other = load_css_paths(x).unwrap(); + let origin = load_css_paths(origin).unwrap(); + let against = load_css_paths(against).unwrap(); let mut ret = Vec::new(); - get_differences(&against, &other, &mut ret); + get_differences(&against, &origin, &mut ret); assert!(ret.is_empty()); - get_differences(&other, &against, &mut ret); + get_differences(&origin, &against, &mut ret); assert_eq!(ret, vec![" Missing rule `c`".to_owned()]); } @@ -123,13 +123,49 @@ fn test_media() { x: y; } } + +@media (max-width: 1001px) { + b { + x: y; + } +} "#; let paths = load_css_paths(text).unwrap(); - eprintln!("{:?}", paths); let p = paths.get("@media (min-width:701px)"); assert!(p.is_some()); let p = p.unwrap(); assert!(p.children.get("a:hover").is_some()); assert!(p.children.get("b").is_some()); + + eprintln!("{:?}", paths); + let p = paths.get("@media (max-width:1001px)"); + assert!(p.is_some()); + let p = p.unwrap(); + assert!(p.children.get("b").is_some()); +} + +#[test] +fn test_css_variables() { + let x = r#" +:root { + --a: #fff; +} +"#; + + let y = r#" +:root { + --a: #fff; + --b: #fff; +} +"#; + + let against = load_css_paths(x).unwrap(); + let other = load_css_paths(y).unwrap(); + + let mut ret = Vec::new(); + get_differences(&against, &other, &mut ret); + assert!(ret.is_empty()); + get_differences(&other, &against, &mut ret); + assert_eq!(ret, vec![" Missing CSS variable `--b` in `:root`".to_owned()]); } From a528f68e79f464f47fd41dd503bbb14e8f34a8ea Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Sep 2022 20:23:19 +0200 Subject: [PATCH 05/15] Remove duplicate warnings --- src/librustdoc/theme.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/librustdoc/theme.rs b/src/librustdoc/theme.rs index 87cfe78e10cbd..ce0cb6c4559a3 100644 --- a/src/librustdoc/theme.rs +++ b/src/librustdoc/theme.rs @@ -133,7 +133,6 @@ fn parse_rules( } match rules.entry(rule) { Entry::Occupied(mut o) => { - eprintln!("Duplicated rule `{}` in CSS selector `{selector}`", o.key()); *o.get_mut() = value; } Entry::Vacant(v) => { @@ -147,7 +146,6 @@ fn parse_rules( match paths.entry(selector) { Entry::Occupied(mut o) => { - eprintln!("Duplicated CSS selector: `{}`", o.key()); let v = o.get_mut(); for (key, value) in rules.into_iter() { v.rules.insert(key, value); From 669f2d4550602bdb41d569fc867b18680b3a495f Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 14 Sep 2022 17:14:36 -0300 Subject: [PATCH 06/15] Revert "Rollup merge of #101496 - spastorino:lower_lifetime_binder_api_changes, r=oli-obk" This reverts commit 953a6b3da7016d41816951ad0930922f558c16d0, reversing changes made to b5ffbd32d4838a460a73ce9aa106a4e1856e52c0. --- compiler/rustc_ast_lowering/src/expr.rs | 64 ++++++++++++------------- compiler/rustc_ast_lowering/src/lib.rs | 53 ++++++++------------ 2 files changed, 51 insertions(+), 66 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index cd03e3fb4572d..2993f1939ea80 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -855,22 +855,21 @@ impl<'hir> LoweringContext<'_, 'hir> { (body_id, generator_option) }); - self.lower_lifetime_binder(closure_id, generic_params, |lctx, bound_generic_params| { - // Lower outside new scope to preserve `is_in_loop_condition`. - let fn_decl = lctx.lower_fn_decl(decl, None, fn_decl_span, FnDeclKind::Closure, None); - - let c = lctx.arena.alloc(hir::Closure { - binder: binder_clause, - capture_clause, - bound_generic_params, - fn_decl, - body: body_id, - fn_decl_span: lctx.lower_span(fn_decl_span), - movability: generator_option, - }); + let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params); + // Lower outside new scope to preserve `is_in_loop_condition`. + let fn_decl = self.lower_fn_decl(decl, None, fn_decl_span, FnDeclKind::Closure, None); + + let c = self.arena.alloc(hir::Closure { + binder: binder_clause, + capture_clause, + bound_generic_params, + fn_decl, + body: body_id, + fn_decl_span: self.lower_span(fn_decl_span), + movability: generator_option, + }); - hir::ExprKind::Closure(c) - }) + hir::ExprKind::Closure(c) } fn generator_movability_for_fn( @@ -957,24 +956,23 @@ impl<'hir> LoweringContext<'_, 'hir> { body_id }); - self.lower_lifetime_binder(closure_id, generic_params, |lctx, bound_generic_params| { - // We need to lower the declaration outside the new scope, because we - // have to conserve the state of being inside a loop condition for the - // closure argument types. - let fn_decl = - lctx.lower_fn_decl(&outer_decl, None, fn_decl_span, FnDeclKind::Closure, None); - - let c = lctx.arena.alloc(hir::Closure { - binder: binder_clause, - capture_clause, - bound_generic_params, - fn_decl, - body, - fn_decl_span: lctx.lower_span(fn_decl_span), - movability: None, - }); - hir::ExprKind::Closure(c) - }) + let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params); + // We need to lower the declaration outside the new scope, because we + // have to conserve the state of being inside a loop condition for the + // closure argument types. + let fn_decl = + self.lower_fn_decl(&outer_decl, None, fn_decl_span, FnDeclKind::Closure, None); + + let c = self.arena.alloc(hir::Closure { + binder: binder_clause, + capture_clause, + bound_generic_params, + fn_decl, + body, + fn_decl_span: self.lower_span(fn_decl_span), + movability: None, + }); + hir::ExprKind::Closure(c) } /// Destructure the LHS of complex assignments. diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 409ee55268cad..c96e419566da1 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -839,31 +839,23 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// name resolver owing to lifetime elision; this also populates the resolver's node-id->def-id /// map, so that later calls to `opt_node_id_to_def_id` that refer to these extra lifetime /// parameters will be successful. - #[instrument(level = "debug", skip(self, in_binder))] + #[instrument(level = "debug", skip(self))] #[inline] - fn lower_lifetime_binder( + fn lower_lifetime_binder( &mut self, binder: NodeId, generic_params: &[GenericParam], - in_binder: impl FnOnce(&mut Self, &'hir [hir::GenericParam<'hir>]) -> R, - ) -> R { + ) -> &'hir [hir::GenericParam<'hir>] { + let mut generic_params: Vec<_> = self.lower_generic_params_mut(generic_params).collect(); let extra_lifetimes = self.resolver.take_extra_lifetime_params(binder); debug!(?extra_lifetimes); - let extra_lifetimes: Vec<_> = extra_lifetimes - .into_iter() - .filter_map(|(ident, node_id, res)| { - self.lifetime_res_to_generic_param(ident, node_id, res) - }) - .collect(); - - let generic_params: Vec<_> = self - .lower_generic_params_mut(generic_params) - .chain(extra_lifetimes.into_iter()) - .collect(); + generic_params.extend(extra_lifetimes.into_iter().filter_map(|(ident, node_id, res)| { + self.lifetime_res_to_generic_param(ident, node_id, res) + })); let generic_params = self.arena.alloc_from_iter(generic_params); debug!(?generic_params); - in_binder(self, generic_params) + generic_params } fn with_dyn_type_scope(&mut self, in_scope: bool, f: impl FnOnce(&mut Self) -> T) -> T { @@ -1268,15 +1260,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { hir::TyKind::Rptr(lifetime, self.lower_mt(mt, itctx)) } TyKind::BareFn(ref f) => { - self.lower_lifetime_binder(t.id, &f.generic_params, |lctx, generic_params| { - hir::TyKind::BareFn(lctx.arena.alloc(hir::BareFnTy { - generic_params, - unsafety: lctx.lower_unsafety(f.unsafety), - abi: lctx.lower_extern(f.ext), - decl: lctx.lower_fn_decl(&f.decl, None, t.span, FnDeclKind::Pointer, None), - param_names: lctx.lower_fn_params_to_names(&f.decl), - })) - }) + let generic_params = self.lower_lifetime_binder(t.id, &f.generic_params); + hir::TyKind::BareFn(self.arena.alloc(hir::BareFnTy { + generic_params, + unsafety: self.lower_unsafety(f.unsafety), + abi: self.lower_extern(f.ext), + decl: self.lower_fn_decl(&f.decl, None, t.span, FnDeclKind::Pointer, None), + param_names: self.lower_fn_params_to_names(&f.decl), + })) } TyKind::Never => hir::TyKind::Never, TyKind::Tup(ref tys) => hir::TyKind::Tup( @@ -2246,14 +2237,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { p: &PolyTraitRef, itctx: &mut ImplTraitContext, ) -> hir::PolyTraitRef<'hir> { - self.lower_lifetime_binder( - p.trait_ref.ref_id, - &p.bound_generic_params, - |lctx, bound_generic_params| { - let trait_ref = lctx.lower_trait_ref(&p.trait_ref, itctx); - hir::PolyTraitRef { bound_generic_params, trait_ref, span: lctx.lower_span(p.span) } - }, - ) + let bound_generic_params = + self.lower_lifetime_binder(p.trait_ref.ref_id, &p.bound_generic_params); + let trait_ref = self.lower_trait_ref(&p.trait_ref, itctx); + hir::PolyTraitRef { bound_generic_params, trait_ref, span: self.lower_span(p.span) } } fn lower_mt(&mut self, mt: &MutTy, itctx: &mut ImplTraitContext) -> hir::MutTy<'hir> { From 861055094cb6cf21ab510d5591a3302c9b7260b1 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 14 Sep 2022 17:39:52 -0300 Subject: [PATCH 07/15] Pass ImplTraitContext as &, there's no need for that to be &mut --- compiler/rustc_ast_lowering/src/asm.rs | 2 +- compiler/rustc_ast_lowering/src/block.rs | 7 +- compiler/rustc_ast_lowering/src/expr.rs | 34 +++++----- compiler/rustc_ast_lowering/src/item.rs | 83 ++++++++++-------------- compiler/rustc_ast_lowering/src/lib.rs | 47 ++++++-------- compiler/rustc_ast_lowering/src/path.rs | 18 +++-- 6 files changed, 84 insertions(+), 107 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/asm.rs b/compiler/rustc_ast_lowering/src/asm.rs index 90bb01aa2165d..24672efc63c55 100644 --- a/compiler/rustc_ast_lowering/src/asm.rs +++ b/compiler/rustc_ast_lowering/src/asm.rs @@ -220,7 +220,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { &sym.qself, &sym.path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); hir::InlineAsmOperand::SymStatic { path, def_id } } else { diff --git a/compiler/rustc_ast_lowering/src/block.rs b/compiler/rustc_ast_lowering/src/block.rs index 7465706d1a9bb..e0869bb1063d9 100644 --- a/compiler/rustc_ast_lowering/src/block.rs +++ b/compiler/rustc_ast_lowering/src/block.rs @@ -84,9 +84,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } fn lower_local(&mut self, l: &Local) -> &'hir hir::Local<'hir> { - let ty = l.ty.as_ref().map(|t| { - self.lower_ty(t, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Variable)) - }); + let ty = l + .ty + .as_ref() + .map(|t| self.lower_ty(t, &ImplTraitContext::Disallowed(ImplTraitPosition::Variable))); let init = l.kind.init().map(|init| self.lower_expr(init)); let hir_id = self.lower_node_id(l.id); let pat = self.lower_pat(&l.pat); diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 2993f1939ea80..506b69f8789b9 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -66,7 +66,7 @@ impl<'hir> LoweringContext<'_, 'hir> { seg, ParamMode::Optional, ParenthesizedGenericArgs::Err, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), )); let receiver = self.lower_expr(receiver); let args = @@ -89,14 +89,14 @@ impl<'hir> LoweringContext<'_, 'hir> { } ExprKind::Cast(ref expr, ref ty) => { let expr = self.lower_expr(expr); - let ty = self - .lower_ty(ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = + self.lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); hir::ExprKind::Cast(expr, ty) } ExprKind::Type(ref expr, ref ty) => { let expr = self.lower_expr(expr); - let ty = self - .lower_ty(ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = + self.lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); hir::ExprKind::Type(expr, ty) } ExprKind::AddrOf(k, m, ref ohs) => { @@ -225,7 +225,7 @@ impl<'hir> LoweringContext<'_, 'hir> { qself, path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); hir::ExprKind::Path(qpath) } @@ -259,7 +259,7 @@ impl<'hir> LoweringContext<'_, 'hir> { &se.qself, &se.path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), )), self.arena .alloc_from_iter(se.fields.iter().map(|x| self.lower_expr_field(x))), @@ -556,14 +556,12 @@ impl<'hir> LoweringContext<'_, 'hir> { async_gen_kind: hir::AsyncGeneratorKind, body: impl FnOnce(&mut Self) -> hir::Expr<'hir>, ) -> hir::ExprKind<'hir> { - let output = - match ret_ty { - Some(ty) => hir::FnRetTy::Return(self.lower_ty( - &ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::AsyncBlock), - )), - None => hir::FnRetTy::DefaultReturn(self.lower_span(span)), - }; + let output = match ret_ty { + Some(ty) => hir::FnRetTy::Return( + self.lower_ty(&ty, &ImplTraitContext::Disallowed(ImplTraitPosition::AsyncBlock)), + ), + None => hir::FnRetTy::DefaultReturn(self.lower_span(span)), + }; // Resume argument type. We let the compiler infer this to simplify the lowering. It is // fully constrained by `future::from_generator`. @@ -1131,7 +1129,7 @@ impl<'hir> LoweringContext<'_, 'hir> { qself, path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); // Destructure like a tuple struct. let tuple_struct_pat = hir::PatKind::TupleStruct( @@ -1150,7 +1148,7 @@ impl<'hir> LoweringContext<'_, 'hir> { qself, path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); // Destructure like a unit struct. let unit_struct_pat = hir::PatKind::Path(qpath); @@ -1174,7 +1172,7 @@ impl<'hir> LoweringContext<'_, 'hir> { &se.qself, &se.path, ParamMode::Optional, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); let fields_omitted = match &se.rest { StructRest::Base(e) => { diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 52273778dcc00..550833275e441 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -313,8 +313,8 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, ty) = self.lower_generics( &generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), - |this| this.lower_ty(ty, &mut ImplTraitContext::TypeAliasesOpaqueTy), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + |this| this.lower_ty(ty, &ImplTraitContext::TypeAliasesOpaqueTy), ); hir::ItemKind::TyAlias(ty, generics) } @@ -326,7 +326,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, ty) = self.lower_generics( &generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| this.arena.alloc(this.ty(span, hir::TyKind::Err)), ); hir::ItemKind::TyAlias(ty, generics) @@ -335,7 +335,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, variants) = self.lower_generics( generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| { this.arena.alloc_from_iter( enum_definition.variants.iter().map(|x| this.lower_variant(x)), @@ -348,7 +348,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, struct_def) = self.lower_generics( generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| this.lower_variant_data(hir_id, struct_def), ); hir::ItemKind::Struct(struct_def, generics) @@ -357,7 +357,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, vdata) = self.lower_generics( generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| this.lower_variant_data(hir_id, vdata), ); hir::ItemKind::Union(vdata, generics) @@ -391,14 +391,12 @@ impl<'hir> LoweringContext<'_, 'hir> { let trait_ref = trait_ref.as_ref().map(|trait_ref| { this.lower_trait_ref( trait_ref, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Trait), + &ImplTraitContext::Disallowed(ImplTraitPosition::Trait), ) }); - let lowered_ty = this.lower_ty( - ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type), - ); + let lowered_ty = this + .lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); (trait_ref, lowered_ty) }); @@ -437,11 +435,11 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, (unsafety, items, bounds)) = self.lower_generics( generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| { let bounds = this.lower_param_bounds( bounds, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Bound), + &ImplTraitContext::Disallowed(ImplTraitPosition::Bound), ); let items = this.arena.alloc_from_iter( items.iter().map(|item| this.lower_trait_item_ref(item)), @@ -456,11 +454,11 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, bounds) = self.lower_generics( generics, id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| { this.lower_param_bounds( bounds, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Bound), + &ImplTraitContext::Disallowed(ImplTraitPosition::Bound), ) }, ); @@ -483,7 +481,7 @@ impl<'hir> LoweringContext<'_, 'hir> { span: Span, body: Option<&Expr>, ) -> (&'hir hir::Ty<'hir>, hir::BodyId) { - let ty = self.lower_ty(ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = self.lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); (ty, self.lower_const_body(span, body)) } @@ -675,8 +673,8 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ForeignItemKind::Fn(fn_dec, fn_args, generics) } ForeignItemKind::Static(ref t, m, _) => { - let ty = self - .lower_ty(t, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = + self.lower_ty(t, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); hir::ForeignItemKind::Static(ty, m) } ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type, @@ -744,11 +742,11 @@ impl<'hir> LoweringContext<'_, 'hir> { qself, path, ParamMode::ExplicitNamed, // no `'_` in declarations (Issue #61124) - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ); self.arena.alloc(t) } else { - self.lower_ty(&f.ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)) + self.lower_ty(&f.ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)) }; let hir_id = self.lower_node_id(f.id); self.lower_attrs(hir_id, &f.attrs); @@ -771,8 +769,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, kind, has_default) = match i.kind { AssocItemKind::Const(_, ref ty, ref default) => { - let ty = - self.lower_ty(ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = self.lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); let body = default.as_ref().map(|x| self.lower_const_body(i.span, Some(x))); (hir::Generics::empty(), hir::TraitItemKind::Const(ty, body), body.is_some()) } @@ -813,18 +810,15 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, kind) = self.lower_generics( &generics, i.id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| { let ty = ty.as_ref().map(|x| { - this.lower_ty( - x, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type), - ) + this.lower_ty(x, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)) }); hir::TraitItemKind::Type( this.lower_param_bounds( bounds, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), ), ty, ) @@ -877,8 +871,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (generics, kind) = match &i.kind { AssocItemKind::Const(_, ty, expr) => { - let ty = - self.lower_ty(ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = self.lower_ty(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); ( hir::Generics::empty(), hir::ImplItemKind::Const(ty, self.lower_const_body(i.span, expr.as_deref())), @@ -905,14 +898,14 @@ impl<'hir> LoweringContext<'_, 'hir> { self.lower_generics( &generics, i.id, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Generic), + &ImplTraitContext::Disallowed(ImplTraitPosition::Generic), |this| match ty { None => { let ty = this.arena.alloc(this.ty(i.span, hir::TyKind::Err)); hir::ImplItemKind::TyAlias(ty) } Some(ty) => { - let ty = this.lower_ty(ty, &mut ImplTraitContext::TypeAliasesOpaqueTy); + let ty = this.lower_ty(ty, &ImplTraitContext::TypeAliasesOpaqueTy); hir::ImplItemKind::TyAlias(ty) } }, @@ -1322,7 +1315,7 @@ impl<'hir> LoweringContext<'_, 'hir> { &mut self, generics: &Generics, parent_node_id: NodeId, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, f: impl FnOnce(&mut Self) -> T, ) -> (&'hir hir::Generics<'hir>, T) { debug_assert!(self.impl_trait_defs.is_empty()); @@ -1427,7 +1420,7 @@ impl<'hir> LoweringContext<'_, 'hir> { id: NodeId, kind: &GenericParamKind, bounds: &[GenericBound], - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, origin: PredicateOrigin, ) -> Option> { // Do not create a clause if we do not have anything inside it. @@ -1502,14 +1495,12 @@ impl<'hir> LoweringContext<'_, 'hir> { span, }) => hir::WherePredicate::BoundPredicate(hir::WhereBoundPredicate { bound_generic_params: self.lower_generic_params(bound_generic_params), - bounded_ty: self.lower_ty( - bounded_ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type), - ), + bounded_ty: self + .lower_ty(bounded_ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)), bounds: self.arena.alloc_from_iter(bounds.iter().map(|bound| { self.lower_param_bound( bound, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Bound), + &ImplTraitContext::Disallowed(ImplTraitPosition::Bound), ) })), span: self.lower_span(span), @@ -1524,20 +1515,16 @@ impl<'hir> LoweringContext<'_, 'hir> { lifetime: self.lower_lifetime(lifetime), bounds: self.lower_param_bounds( bounds, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Bound), + &ImplTraitContext::Disallowed(ImplTraitPosition::Bound), ), in_where_clause: true, }), WherePredicate::EqPredicate(WhereEqPredicate { ref lhs_ty, ref rhs_ty, span }) => { hir::WherePredicate::EqPredicate(hir::WhereEqPredicate { - lhs_ty: self.lower_ty( - lhs_ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type), - ), - rhs_ty: self.lower_ty( - rhs_ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type), - ), + lhs_ty: self + .lower_ty(lhs_ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)), + rhs_ty: self + .lower_ty(rhs_ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)), span: self.lower_span(span), }) } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index c96e419566da1..bda80c3eb192e 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -984,7 +984,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_assoc_ty_constraint( &mut self, constraint: &AssocConstraint, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::TypeBinding<'hir> { debug!("lower_assoc_ty_constraint(constraint={:?}, itctx={:?})", constraint, itctx); // lower generic arguments of identifier in constraint @@ -1003,7 +1003,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } else { self.arena.alloc(hir::GenericArgs::none()) }; - let mut itctx_tait = ImplTraitContext::TypeAliasesOpaqueTy; + let itctx_tait = &ImplTraitContext::TypeAliasesOpaqueTy; let kind = match constraint.kind { AssocConstraintKind::Equality { ref term } => { @@ -1041,9 +1041,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // then to an opaque type). // // FIXME: this is only needed until `impl Trait` is allowed in type aliases. - ImplTraitContext::Disallowed(_) if self.is_in_dyn_type => { - (true, &mut itctx_tait) - } + ImplTraitContext::Disallowed(_) if self.is_in_dyn_type => (true, itctx_tait), // We are in the parameter position, but not within a dyn type: // @@ -1122,7 +1120,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_generic_arg( &mut self, arg: &ast::GenericArg, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::GenericArg<'hir> { match arg { ast::GenericArg::Lifetime(lt) => GenericArg::Lifetime(self.lower_lifetime(<)), @@ -1184,7 +1182,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } #[instrument(level = "debug", skip(self))] - fn lower_ty(&mut self, t: &Ty, itctx: &mut ImplTraitContext) -> &'hir hir::Ty<'hir> { + fn lower_ty(&mut self, t: &Ty, itctx: &ImplTraitContext) -> &'hir hir::Ty<'hir> { self.arena.alloc(self.lower_ty_direct(t, itctx)) } @@ -1194,7 +1192,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { qself: &Option, path: &Path, param_mode: ParamMode, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::Ty<'hir> { // Check whether we should interpret this as a bare trait object. // This check mirrors the one in late resolution. We only introduce this special case in @@ -1237,7 +1235,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { self.ty(span, hir::TyKind::Tup(tys)) } - fn lower_ty_direct(&mut self, t: &Ty, itctx: &mut ImplTraitContext) -> hir::Ty<'hir> { + fn lower_ty_direct(&mut self, t: &Ty, itctx: &ImplTraitContext) -> hir::Ty<'hir> { let kind = match t.kind { TyKind::Infer => hir::TyKind::Infer, TyKind::Err => hir::TyKind::Err, @@ -1348,7 +1346,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { def_node_id, bounds, false, - &mut ImplTraitContext::TypeAliasesOpaqueTy, + &ImplTraitContext::TypeAliasesOpaqueTy, ), ImplTraitContext::Universal => { let span = t.span; @@ -1435,7 +1433,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { opaque_ty_node_id: NodeId, bounds: &GenericBounds, in_trait: bool, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::TyKind<'hir> { // Make sure we know that some funky desugaring has been going on here. // This is a first: there is code in other places like for loop @@ -1681,11 +1679,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } let inputs = self.arena.alloc_from_iter(inputs.iter().map(|param| { if fn_node_id.is_some() { - self.lower_ty_direct(¶m.ty, &mut ImplTraitContext::Universal) + self.lower_ty_direct(¶m.ty, &ImplTraitContext::Universal) } else { self.lower_ty_direct( ¶m.ty, - &mut ImplTraitContext::Disallowed(match kind { + &ImplTraitContext::Disallowed(match kind { FnDeclKind::Fn | FnDeclKind::Inherent => { unreachable!("fn should allow in-band lifetimes") } @@ -2084,7 +2082,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_param_bound( &mut self, tpb: &GenericBound, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::GenericBound<'hir> { match tpb { GenericBound::Trait(p, modifier) => hir::GenericBound::Trait( @@ -2200,7 +2198,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { GenericParamKind::Type { ref default, .. } => { let kind = hir::GenericParamKind::Type { default: default.as_ref().map(|x| { - self.lower_ty(x, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)) + self.lower_ty(x, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)) }), synthetic: false, }; @@ -2208,8 +2206,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { (hir::ParamName::Plain(self.lower_ident(param.ident)), kind) } GenericParamKind::Const { ref ty, kw_span: _, ref default } => { - let ty = - self.lower_ty(&ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::Type)); + let ty = self.lower_ty(&ty, &ImplTraitContext::Disallowed(ImplTraitPosition::Type)); let default = default.as_ref().map(|def| self.lower_anon_const(def)); ( hir::ParamName::Plain(self.lower_ident(param.ident)), @@ -2219,11 +2216,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } - fn lower_trait_ref( - &mut self, - p: &TraitRef, - itctx: &mut ImplTraitContext, - ) -> hir::TraitRef<'hir> { + fn lower_trait_ref(&mut self, p: &TraitRef, itctx: &ImplTraitContext) -> hir::TraitRef<'hir> { let path = match self.lower_qpath(p.ref_id, &None, &p.path, ParamMode::Explicit, itctx) { hir::QPath::Resolved(None, path) => path, qpath => panic!("lower_trait_ref: unexpected QPath `{:?}`", qpath), @@ -2235,7 +2228,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_poly_trait_ref( &mut self, p: &PolyTraitRef, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::PolyTraitRef<'hir> { let bound_generic_params = self.lower_lifetime_binder(p.trait_ref.ref_id, &p.bound_generic_params); @@ -2243,7 +2236,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { hir::PolyTraitRef { bound_generic_params, trait_ref, span: self.lower_span(p.span) } } - fn lower_mt(&mut self, mt: &MutTy, itctx: &mut ImplTraitContext) -> hir::MutTy<'hir> { + fn lower_mt(&mut self, mt: &MutTy, itctx: &ImplTraitContext) -> hir::MutTy<'hir> { hir::MutTy { ty: self.lower_ty(&mt.ty, itctx), mutbl: mt.mutbl } } @@ -2251,7 +2244,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_param_bounds( &mut self, bounds: &[GenericBound], - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::GenericBounds<'hir> { self.arena.alloc_from_iter(self.lower_param_bounds_mut(bounds, itctx)) } @@ -2259,7 +2252,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_param_bounds_mut<'s, 'b>( &'s mut self, bounds: &'s [GenericBound], - itctx: &'b mut ImplTraitContext, + itctx: &'b ImplTraitContext, ) -> impl Iterator> + Captures<'s> + Captures<'a> + Captures<'b> { bounds.iter().map(move |bound| self.lower_param_bound(bound, itctx)) @@ -2291,7 +2284,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { node_id, &GenericParamKind::Type { default: None }, bounds, - &mut ImplTraitContext::Universal, + &ImplTraitContext::Universal, hir::PredicateOrigin::ImplTrait, ); diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index a3d864023a9ff..6bb1bb9eace8b 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/compiler/rustc_ast_lowering/src/path.rs @@ -22,7 +22,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { qself: &Option, p: &Path, param_mode: ParamMode, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::QPath<'hir> { let qself_position = qself.as_ref().map(|q| q.position); let qself = qself.as_ref().map(|q| self.lower_ty(&q.ty, itctx)); @@ -156,7 +156,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { segment, param_mode, ParenthesizedGenericArgs::Err, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::Path), + &ImplTraitContext::Disallowed(ImplTraitPosition::Path), ) })), span: self.lower_span(p.span), @@ -180,7 +180,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { segment: &PathSegment, param_mode: ParamMode, parenthesized_generic_args: ParenthesizedGenericArgs, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> hir::PathSegment<'hir> { debug!("path_span: {:?}, lower_path_segment(segment: {:?})", path_span, segment,); let (mut generic_args, infer_args) = if let Some(ref generic_args) = segment.args { @@ -316,7 +316,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { &mut self, data: &AngleBracketedArgs, param_mode: ParamMode, - itctx: &mut ImplTraitContext, + itctx: &ImplTraitContext, ) -> (GenericArgsCtor<'hir>, bool) { let has_non_lt_args = data.args.iter().any(|arg| match arg { AngleBracketedArg::Arg(ast::GenericArg::Lifetime(_)) @@ -350,14 +350,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // we generally don't permit such things (see #51008). let ParenthesizedArgs { span, inputs, inputs_span, output } = data; let inputs = self.arena.alloc_from_iter(inputs.iter().map(|ty| { - self.lower_ty_direct( - ty, - &mut ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitParam), - ) + self.lower_ty_direct(ty, &ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitParam)) })); let output_ty = match output { - FnRetTy::Ty(ty) => self - .lower_ty(&ty, &mut ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitReturn)), + FnRetTy::Ty(ty) => { + self.lower_ty(&ty, &ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitReturn)) + } FnRetTy::Default(_) => self.arena.alloc(self.ty_tup(*span, &[])), }; let args = smallvec![GenericArg::Type(self.arena.alloc(self.ty_tup(*inputs_span, inputs)))]; From 45d8049387e944bcfc31fcf72a7924f7067a7644 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Wed, 14 Sep 2022 17:40:51 -0300 Subject: [PATCH 08/15] Get rid of 'b lifetime in lower_param_bounds_mut --- compiler/rustc_ast_lowering/src/lib.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index bda80c3eb192e..cb6bf0863b387 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -2249,12 +2249,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { self.arena.alloc_from_iter(self.lower_param_bounds_mut(bounds, itctx)) } - fn lower_param_bounds_mut<'s, 'b>( + fn lower_param_bounds_mut<'s>( &'s mut self, bounds: &'s [GenericBound], - itctx: &'b ImplTraitContext, - ) -> impl Iterator> + Captures<'s> + Captures<'a> + Captures<'b> - { + itctx: &'s ImplTraitContext, + ) -> impl Iterator> + Captures<'s> + Captures<'a> { bounds.iter().map(move |bound| self.lower_param_bound(bound, itctx)) } From 98e20c097cbb402ee2c6f4056fe8b4acca9679f9 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 15 Sep 2022 15:18:23 +0800 Subject: [PATCH 09/15] fix #101797: Suggest associated const for incorrect use of let in traits --- compiler/rustc_parse/src/parser/item.rs | 19 +++++++++++++++---- src/test/ui/parser/suggest-assoc-const.fixed | 10 ++++++++++ src/test/ui/parser/suggest-assoc-const.rs | 10 ++++++++++ src/test/ui/parser/suggest-assoc-const.stderr | 8 ++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/parser/suggest-assoc-const.fixed create mode 100644 src/test/ui/parser/suggest-assoc-const.rs create mode 100644 src/test/ui/parser/suggest-assoc-const.stderr diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index b190a7062defd..045dc7357f177 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -698,11 +698,22 @@ impl<'a> Parser<'a> { let semicolon_span = self.token.span; // We have to bail or we'll potentially never make progress. let non_item_span = self.token.span; - self.consume_block(Delimiter::Brace, ConsumeClosingDelim::Yes); + let is_let = self.token.is_keyword(kw::Let); + let mut err = self.struct_span_err(non_item_span, "non-item in item list"); - err.span_label(open_brace_span, "item list starts here") - .span_label(non_item_span, "non-item starts here") - .span_label(self.prev_token.span, "item list ends here"); + self.consume_block(Delimiter::Brace, ConsumeClosingDelim::Yes); + if is_let { + err.span_suggestion( + non_item_span, + "considering use `const` instead of `let` for associated const", + "const", + Applicability::MachineApplicable, + ); + } else { + err.span_label(open_brace_span, "item list starts here") + .span_label(non_item_span, "non-item starts here") + .span_label(self.prev_token.span, "item list ends here"); + } if is_unnecessary_semicolon { err.span_suggestion( semicolon_span, diff --git a/src/test/ui/parser/suggest-assoc-const.fixed b/src/test/ui/parser/suggest-assoc-const.fixed new file mode 100644 index 0000000000000..259f37b23a584 --- /dev/null +++ b/src/test/ui/parser/suggest-assoc-const.fixed @@ -0,0 +1,10 @@ +// Issue: 101797, Suggest associated const for incorrect use of let in traits +// run-rustfix +trait Trait { + const _X: i32; + //~^ ERROR non-item in item list +} + +fn main() { + +} diff --git a/src/test/ui/parser/suggest-assoc-const.rs b/src/test/ui/parser/suggest-assoc-const.rs new file mode 100644 index 0000000000000..c7be712ec076e --- /dev/null +++ b/src/test/ui/parser/suggest-assoc-const.rs @@ -0,0 +1,10 @@ +// Issue: 101797, Suggest associated const for incorrect use of let in traits +// run-rustfix +trait Trait { + let _X: i32; + //~^ ERROR non-item in item list +} + +fn main() { + +} diff --git a/src/test/ui/parser/suggest-assoc-const.stderr b/src/test/ui/parser/suggest-assoc-const.stderr new file mode 100644 index 0000000000000..b92e399c90542 --- /dev/null +++ b/src/test/ui/parser/suggest-assoc-const.stderr @@ -0,0 +1,8 @@ +error: non-item in item list + --> $DIR/suggest-assoc-const.rs:4:5 + | +LL | let _X: i32; + | ^^^ help: considering use `const` instead of `let` for associated const: `const` + +error: aborting due to previous error + From d3529ceb6c5eeaaecd3d88e97ad17740258772a8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 15 Sep 2022 13:53:20 +0200 Subject: [PATCH 10/15] Correctly handle parens --- src/librustdoc/theme.rs | 28 +++++++++++++++++++++------- src/librustdoc/theme/tests.rs | 18 +++++++++++++++++- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/theme.rs b/src/librustdoc/theme.rs index ce0cb6c4559a3..e7a26cb346ee6 100644 --- a/src/librustdoc/theme.rs +++ b/src/librustdoc/theme.rs @@ -17,19 +17,31 @@ pub(crate) struct CssPath { } /// When encountering a `"` or a `'`, returns the whole string, including the quote characters. -fn get_string(iter: &mut Peekable>, string_start: char) -> String { - let mut s = String::with_capacity(2); - - s.push(string_start); +fn get_string(iter: &mut Peekable>, string_start: char, buffer: &mut String) { + buffer.push(string_start); while let Some(c) = iter.next() { - s.push(c); + buffer.push(c); if c == '\\' { iter.next(); } else if c == string_start { break; } } - s +} + +fn get_inside_paren( + iter: &mut Peekable>, + paren_start: char, + paren_end: char, + buffer: &mut String, +) { + buffer.push(paren_start); + while let Some(c) = iter.next() { + handle_common_chars(c, buffer, iter); + if c == paren_end { + break; + } + } } /// Skips a `/*` comment. @@ -52,9 +64,11 @@ fn skip_line_comment(iter: &mut Peekable>) { fn handle_common_chars(c: char, buffer: &mut String, iter: &mut Peekable>) { match c { - '"' | '\'' => buffer.push_str(&get_string(iter, c)), + '"' | '\'' => get_string(iter, c, buffer), '/' if iter.peek() == Some(&'*') => skip_comment(iter), '/' if iter.peek() == Some(&'/') => skip_line_comment(iter), + '(' => get_inside_paren(iter, c, ')', buffer), + '[' => get_inside_paren(iter, c, ']', buffer), _ => buffer.push(c), } } diff --git a/src/librustdoc/theme/tests.rs b/src/librustdoc/theme/tests.rs index ce1d77d98d7ad..08a174d27d357 100644 --- a/src/librustdoc/theme/tests.rs +++ b/src/librustdoc/theme/tests.rs @@ -138,7 +138,6 @@ fn test_media() { assert!(p.children.get("a:hover").is_some()); assert!(p.children.get("b").is_some()); - eprintln!("{:?}", paths); let p = paths.get("@media (max-width:1001px)"); assert!(p.is_some()); let p = p.unwrap(); @@ -169,3 +168,20 @@ fn test_css_variables() { get_differences(&other, &against, &mut ret); assert_eq!(ret, vec![" Missing CSS variable `--b` in `:root`".to_owned()]); } + +#[test] +fn test_weird_rule_value() { + let x = r#" +a[text=("a")] { + b: url({;}.png); + c: #fff +} +"#; + + let paths = load_css_paths(&x).unwrap(); + let p = paths.get("a[text=(\"a\")]"); + assert!(p.is_some()); + let p = p.unwrap(); + assert_eq!(p.rules.get("b"), Some(&"url({;}.png)".to_owned())); + assert_eq!(p.rules.get("c"), Some(&"#fff".to_owned())); +} From 6d7beafc87e7cd8806987c5203e52bc3b8c458fa Mon Sep 17 00:00:00 2001 From: Rageking8 Date: Thu, 15 Sep 2022 23:51:43 +0800 Subject: [PATCH 11/15] slight vertical formatting --- .../rustc_parse/src/parser/nonterminal.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index e215b6872bff8..103dd801257a7 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -66,18 +66,18 @@ impl<'a> Parser<'a> { }, NonterminalKind::PatParam { .. } | NonterminalKind::PatWithOr { .. } => { match token.kind { - token::Ident(..) | // box, ref, mut, and other identifiers (can stricten) - token::OpenDelim(Delimiter::Parenthesis) | // tuple pattern - token::OpenDelim(Delimiter::Bracket) | // slice pattern - token::BinOp(token::And) | // reference - token::BinOp(token::Minus) | // negative literal - token::AndAnd | // double reference - token::Literal(..) | // literal - token::DotDot | // range pattern (future compat) - token::DotDotDot | // range pattern (future compat) - token::ModSep | // path - token::Lt | // path (UFCS constant) - token::BinOp(token::Shl) => true, // path (double UFCS) + token::Ident(..) | // box, ref, mut, and other identifiers (can stricten) + token::OpenDelim(Delimiter::Parenthesis) | // tuple pattern + token::OpenDelim(Delimiter::Bracket) | // slice pattern + token::BinOp(token::And) | // reference + token::BinOp(token::Minus) | // negative literal + token::AndAnd | // double reference + token::Literal(..) | // literal + token::DotDot | // range pattern (future compat) + token::DotDotDot | // range pattern (future compat) + token::ModSep | // path + token::Lt | // path (UFCS constant) + token::BinOp(token::Shl) => true, // path (double UFCS) // leading vert `|` or-pattern token::BinOp(token::Or) => matches!(kind, NonterminalKind::PatWithOr {..}), token::Interpolated(ref nt) => may_be_ident(nt), From 4bf7d2ca913179e3d4c71871a112820624c1e379 Mon Sep 17 00:00:00 2001 From: yukang Date: Fri, 16 Sep 2022 01:09:26 +0800 Subject: [PATCH 12/15] tweak suggestion --- compiler/rustc_parse/src/parser/item.rs | 2 +- src/test/ui/parser/suggest-assoc-const.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 045dc7357f177..e55b5ce71cded 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -705,7 +705,7 @@ impl<'a> Parser<'a> { if is_let { err.span_suggestion( non_item_span, - "considering use `const` instead of `let` for associated const", + "consider using `const` instead of `let` for associated const", "const", Applicability::MachineApplicable, ); diff --git a/src/test/ui/parser/suggest-assoc-const.stderr b/src/test/ui/parser/suggest-assoc-const.stderr index b92e399c90542..2ddfa07c5be47 100644 --- a/src/test/ui/parser/suggest-assoc-const.stderr +++ b/src/test/ui/parser/suggest-assoc-const.stderr @@ -2,7 +2,7 @@ error: non-item in item list --> $DIR/suggest-assoc-const.rs:4:5 | LL | let _X: i32; - | ^^^ help: considering use `const` instead of `let` for associated const: `const` + | ^^^ help: consider using `const` instead of `let` for associated const: `const` error: aborting due to previous error From ef24747703b6a30abb242d43f09c584edfed1b24 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 15 Sep 2022 13:03:04 -0700 Subject: [PATCH 13/15] rustdoc: use more precise URLs for jump-to-definition links As an example, this cuts down by about 11%. $ du -h new_mod.rs.html old_mod.rs.html 296K new_mod.rs.html 332K old_mod.rs.html --- src/librustdoc/html/highlight.rs | 8 ++++--- src/librustdoc/html/render/context.rs | 30 +++++++++++++++++++++++++++ src/librustdoc/html/sources.rs | 5 ++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 1e6cab8fcd3b7..8922bf377858e 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -29,6 +29,8 @@ pub(crate) struct HrefContext<'a, 'b, 'c> { /// This field is used to know "how far" from the top of the directory we are to link to either /// documentation pages or other source pages. pub(crate) root_path: &'c str, + /// This field is used to calculate precise local URLs. + pub(crate) current_href: &'c str, } /// Decorations are represented as a map from CSS class to vector of character ranges. @@ -977,9 +979,9 @@ fn string_without_closing_tag( // a link to their definition can be generated using this: // https://github.com/rust-lang/rust/blob/60f1a2fc4b535ead9c85ce085fdce49b1b097531/src/librustdoc/html/render/context.rs#L315-L338 match href { - LinkFromSrc::Local(span) => context - .href_from_span(*span, true) - .map(|s| format!("{}{}", href_context.root_path, s)), + LinkFromSrc::Local(span) => { + context.href_from_span_relative(*span, href_context.current_href) + } LinkFromSrc::External(def_id) => { format::href_with_root_path(*def_id, context, Some(href_context.root_path)) .ok() diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 01b96dc721558..62def4a94e8dc 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -31,6 +31,7 @@ use crate::formats::FormatRenderer; use crate::html::escape::Escape; use crate::html::format::{join_with_double_colon, Buffer}; use crate::html::markdown::{self, plain_text_summary, ErrorCodes, IdMap}; +use crate::html::url_parts_builder::UrlPartsBuilder; use crate::html::{layout, sources}; use crate::scrape_examples::AllCallLocations; use crate::try_err; @@ -370,6 +371,35 @@ impl<'tcx> Context<'tcx> { anchor = anchor )) } + + pub(crate) fn href_from_span_relative( + &self, + span: clean::Span, + relative_to: &str, + ) -> Option { + self.href_from_span(span, false).map(|s| { + let mut url = UrlPartsBuilder::new(); + let mut dest_href_parts = s.split('/'); + let mut cur_href_parts = relative_to.split('/'); + for (cur_href_part, dest_href_part) in (&mut cur_href_parts).zip(&mut dest_href_parts) { + if cur_href_part != dest_href_part { + url.push(dest_href_part); + break; + } + } + for dest_href_part in dest_href_parts { + url.push(dest_href_part); + } + let loline = span.lo(self.sess()).line; + let hiline = span.hi(self.sess()).line; + format!( + "{}{}#{}", + "../".repeat(cur_href_parts.count()), + url.finish(), + if loline == hiline { loline.to_string() } else { format!("{loline}-{hiline}") } + ) + }) + } } /// Generates the documentation for `crate` into the directory `dst` diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index f37c54e42983f..2e2bee78b95f9 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -288,11 +288,14 @@ pub(crate) fn print_src( } } line_numbers.write_str(""); + let current_href = &context + .href_from_span(clean::Span::new(file_span), false) + .expect("only local crates should have sources emitted"); highlight::render_source_with_highlighting( s, buf, line_numbers, - highlight::HrefContext { context, file_span, root_path }, + highlight::HrefContext { context, file_span, root_path, current_href }, decoration_info, ); } From 669498ca0a880e2272fee62bf5dc50f3bc07e75b Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 15 Sep 2022 13:14:30 -0700 Subject: [PATCH 14/15] rustdoc: fix test cases --- .../check-source-code-urls-to-def-std.rs | 8 +++---- .../rustdoc/check-source-code-urls-to-def.rs | 22 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/test/rustdoc/check-source-code-urls-to-def-std.rs b/src/test/rustdoc/check-source-code-urls-to-def-std.rs index 3396b234a77b1..e12d8445f4fa3 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def-std.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def-std.rs @@ -9,7 +9,7 @@ fn babar() {} // @has - '//a[@href="{{channel}}/std/primitive.u32.html"]' 'u32' // @has - '//a[@href="{{channel}}/std/primitive.str.html"]' 'str' // @has - '//a[@href="{{channel}}/std/primitive.bool.html"]' 'bool' -// @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#7"]' 'babar' +// @has - '//a[@href="#7"]' 'babar' pub fn foo(a: u32, b: &str, c: String) { let x = 12; let y: bool = true; @@ -31,12 +31,12 @@ macro_rules! data { pub fn another_foo() { // This is known limitation: if the macro doesn't generate anything, the visitor // can't find any item or anything that could tell us that it comes from expansion. - // @!has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#19"]' 'yolo!' + // @!has - '//a[@href="#19"]' 'yolo!' yolo!(); // @has - '//a[@href="{{channel}}/std/macro.eprintln.html"]' 'eprintln!' eprintln!(); - // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#27-29"]' 'data!' + // @has - '//a[@href="#27-29"]' 'data!' let x = data!(4); - // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#23-25"]' 'bar!' + // @has - '//a[@href="#23-25"]' 'bar!' bar!(x); } diff --git a/src/test/rustdoc/check-source-code-urls-to-def.rs b/src/test/rustdoc/check-source-code-urls-to-def.rs index ec44e1bc65c90..d00a3e3551991 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def.rs @@ -10,14 +10,14 @@ extern crate source_code; // @has 'src/foo/check-source-code-urls-to-def.rs.html' -// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#1-17"]' 'bar' +// @has - '//a[@href="auxiliary/source-code-bar.rs.html#1-17"]' 'bar' #[path = "auxiliary/source-code-bar.rs"] pub mod bar; -// @count - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#5"]' 4 +// @count - '//a[@href="auxiliary/source-code-bar.rs.html#5"]' 4 use bar::Bar; -// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#13"]' 'self' -// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14"]' 'Trait' +// @has - '//a[@href="auxiliary/source-code-bar.rs.html#13"]' 'self' +// @has - '//a[@href="auxiliary/source-code-bar.rs.html#14"]' 'Trait' use bar::sub::{self, Trait}; pub struct Foo; @@ -31,26 +31,26 @@ fn babar() {} // @has - '//a/@href' '/struct.String.html' // @has - '//a/@href' '/primitive.u32.html' // @has - '//a/@href' '/primitive.str.html' -// @count - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#23"]' 5 +// @count - '//a[@href="#23"]' 5 // @has - '//a[@href="../../source_code/struct.SourceCode.html"]' 'source_code::SourceCode' pub fn foo(a: u32, b: &str, c: String, d: Foo, e: bar::Bar, f: source_code::SourceCode) { let x = 12; let y: Foo = Foo; let z: Bar = bar::Bar { field: Foo }; babar(); - // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#26"]' 'hello' + // @has - '//a[@href="#26"]' 'hello' y.hello(); } -// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14"]' 'bar::sub::Trait' -// @has - '//a[@href="../../src/foo/auxiliary/source-code-bar.rs.html#14"]' 'Trait' +// @has - '//a[@href="auxiliary/source-code-bar.rs.html#14"]' 'bar::sub::Trait' +// @has - '//a[@href="auxiliary/source-code-bar.rs.html#14"]' 'Trait' pub fn foo2(t: &T, v: &V, b: bool) {} pub trait AnotherTrait {} pub trait WhyNot {} -// @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#49"]' 'AnotherTrait' -// @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#50"]' 'WhyNot' +// @has - '//a[@href="#49"]' 'AnotherTrait' +// @has - '//a[@href="#50"]' 'WhyNot' pub fn foo3(t: &T, v: &V) where T: AnotherTrait, @@ -59,7 +59,7 @@ where pub trait AnotherTrait2 {} -// @has - '//a[@href="../../src/foo/check-source-code-urls-to-def.rs.html#60"]' 'AnotherTrait2' +// @has - '//a[@href="#60"]' 'AnotherTrait2' pub fn foo4() { let x: Vec = Vec::new(); } From 7c8e4ef49d21e3f79b60f6546cfb5181bb3b2592 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 15 Sep 2022 17:16:59 -0700 Subject: [PATCH 15/15] rustdoc: remove no-op CSS `.block { padding: 0 }` This rule was changed in 8fb1250aba8135679463351a3813c04ae45bf311 from the original version that had a non-zero padding. It's not needed, because it's not overriding anything that would've given `.block` a padding. --- src/librustdoc/html/static/css/rustdoc.css | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 8bfaaf21c8e57..ad1fd738a2037 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -498,9 +498,6 @@ img { font-weight: 500; } -.block { - padding: 0; -} .block ul, .block li { padding: 0; margin: 0;