From c7b6e1de661419a67a59ad59fba70a451c27cbb6 Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 8 Jun 2022 12:39:14 +0200 Subject: [PATCH 01/13] lub: don't bail out due to empty binders --- compiler/rustc_infer/src/infer/glb.rs | 20 +++++-- compiler/rustc_infer/src/infer/lub.rs | 20 +++++-- src/test/ui/lub-glb/empty-binders-err.rs | 61 ++++++++++++++++++++ src/test/ui/lub-glb/empty-binders-err.stderr | 59 +++++++++++++++++++ src/test/ui/lub-glb/empty-binders.rs | 45 +++++++++++++++ 5 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/lub-glb/empty-binders-err.rs create mode 100644 src/test/ui/lub-glb/empty-binders-err.stderr create mode 100644 src/test/ui/lub-glb/empty-binders.rs diff --git a/compiler/rustc_infer/src/infer/glb.rs b/compiler/rustc_infer/src/infer/glb.rs index 0d38b94965afe..1570a08f3ca8b 100644 --- a/compiler/rustc_infer/src/infer/glb.rs +++ b/compiler/rustc_infer/src/infer/glb.rs @@ -95,12 +95,20 @@ impl<'tcx> TypeRelation<'tcx> for Glb<'_, '_, 'tcx> { T: Relate<'tcx>, { debug!("binders(a={:?}, b={:?})", a, b); - - // When higher-ranked types are involved, computing the LUB is - // very challenging, switch to invariance. This is obviously - // overly conservative but works ok in practice. - self.relate_with_variance(ty::Variance::Invariant, ty::VarianceDiagInfo::default(), a, b)?; - Ok(a) + if a.skip_binder().has_escaping_bound_vars() || b.skip_binder().has_escaping_bound_vars() { + // When higher-ranked types are involved, computing the GLB is + // very challenging, switch to invariance. This is obviously + // overly conservative but works ok in practice. + self.relate_with_variance( + ty::Variance::Invariant, + ty::VarianceDiagInfo::default(), + a, + b, + )?; + Ok(a) + } else { + Ok(ty::Binder::dummy(self.relate(a.skip_binder(), b.skip_binder())?)) + } } } diff --git a/compiler/rustc_infer/src/infer/lub.rs b/compiler/rustc_infer/src/infer/lub.rs index 498c1e907c4e1..9f96d52c85034 100644 --- a/compiler/rustc_infer/src/infer/lub.rs +++ b/compiler/rustc_infer/src/infer/lub.rs @@ -95,12 +95,20 @@ impl<'tcx> TypeRelation<'tcx> for Lub<'_, '_, 'tcx> { T: Relate<'tcx>, { debug!("binders(a={:?}, b={:?})", a, b); - - // When higher-ranked types are involved, computing the LUB is - // very challenging, switch to invariance. This is obviously - // overly conservative but works ok in practice. - self.relate_with_variance(ty::Variance::Invariant, ty::VarianceDiagInfo::default(), a, b)?; - Ok(a) + if a.skip_binder().has_escaping_bound_vars() || b.skip_binder().has_escaping_bound_vars() { + // When higher-ranked types are involved, computing the LUB is + // very challenging, switch to invariance. This is obviously + // overly conservative but works ok in practice. + self.relate_with_variance( + ty::Variance::Invariant, + ty::VarianceDiagInfo::default(), + a, + b, + )?; + Ok(a) + } else { + Ok(ty::Binder::dummy(self.relate(a.skip_binder(), b.skip_binder())?)) + } } } diff --git a/src/test/ui/lub-glb/empty-binders-err.rs b/src/test/ui/lub-glb/empty-binders-err.rs new file mode 100644 index 0000000000000..ee28dd7c97d61 --- /dev/null +++ b/src/test/ui/lub-glb/empty-binders-err.rs @@ -0,0 +1,61 @@ +fn lt<'a: 'a>() -> &'a () { + &() +} + +fn lt_in_fn<'a: 'a>() -> fn(&'a ()) { + |_| () +} + +struct Contra<'a>(fn(&'a ())); +fn lt_in_contra<'a: 'a>() -> Contra<'a> { + Contra(|_| ()) +} + +fn covariance<'a, 'b, 'upper, 'lower>(v: bool) +where + 'upper: 'a, + 'upper: 'b, + 'a: 'lower, + 'b: 'lower, + +{ + let _: &'upper () = match v { + //~^ ERROR lifetime may not live long enough + //~| ERROR lifetime may not live long enough + true => lt::<'a>(), + false => lt::<'b>(), + }; +} + +fn contra_fn<'a, 'b, 'upper, 'lower>(v: bool) +where + 'upper: 'a, + 'upper: 'b, + 'a: 'lower, + 'b: 'lower, + +{ + + let _: fn(&'lower ()) = match v { + //~^ ERROR lifetime may not live long enough + true => lt_in_fn::<'a>(), + false => lt_in_fn::<'b>(), + }; +} + +fn contra_struct<'a, 'b, 'upper, 'lower>(v: bool) +where + 'upper: 'a, + 'upper: 'b, + 'a: 'lower, + 'b: 'lower, + +{ + let _: Contra<'lower> = match v { + //~^ ERROR lifetime may not live long enough + true => lt_in_contra::<'a>(), + false => lt_in_contra::<'b>(), + }; +} + +fn main() {} diff --git a/src/test/ui/lub-glb/empty-binders-err.stderr b/src/test/ui/lub-glb/empty-binders-err.stderr new file mode 100644 index 0000000000000..0d5de978e4301 --- /dev/null +++ b/src/test/ui/lub-glb/empty-binders-err.stderr @@ -0,0 +1,59 @@ +error: lifetime may not live long enough + --> $DIR/empty-binders-err.rs:22:12 + | +LL | fn covariance<'a, 'b, 'upper, 'lower>(v: bool) + | -- ------ lifetime `'upper` defined here + | | + | lifetime `'a` defined here +... +LL | let _: &'upper () = match v { + | ^^^^^^^^^^ type annotation requires that `'a` must outlive `'upper` + | + = help: consider adding the following bound: `'a: 'upper` + +error: lifetime may not live long enough + --> $DIR/empty-binders-err.rs:22:12 + | +LL | fn covariance<'a, 'b, 'upper, 'lower>(v: bool) + | -- ------ lifetime `'upper` defined here + | | + | lifetime `'b` defined here +... +LL | let _: &'upper () = match v { + | ^^^^^^^^^^ type annotation requires that `'b` must outlive `'upper` + | + = help: consider adding the following bound: `'b: 'upper` + +help: the following changes may resolve your lifetime errors + | + = help: add bound `'a: 'upper` + = help: add bound `'b: 'upper` + +error: lifetime may not live long enough + --> $DIR/empty-binders-err.rs:39:12 + | +LL | fn contra_fn<'a, 'b, 'upper, 'lower>(v: bool) + | -- ------ lifetime `'lower` defined here + | | + | lifetime `'a` defined here +... +LL | let _: fn(&'lower ()) = match v { + | ^^^^^^^^^^^^^^ type annotation requires that `'lower` must outlive `'a` + | + = help: consider adding the following bound: `'lower: 'a` + +error: lifetime may not live long enough + --> $DIR/empty-binders-err.rs:54:12 + | +LL | fn contra_struct<'a, 'b, 'upper, 'lower>(v: bool) + | -- ------ lifetime `'lower` defined here + | | + | lifetime `'a` defined here +... +LL | let _: Contra<'lower> = match v { + | ^^^^^^^^^^^^^^ type annotation requires that `'lower` must outlive `'a` + | + = help: consider adding the following bound: `'lower: 'a` + +error: aborting due to 4 previous errors + diff --git a/src/test/ui/lub-glb/empty-binders.rs b/src/test/ui/lub-glb/empty-binders.rs new file mode 100644 index 0000000000000..f9d07e79fdabf --- /dev/null +++ b/src/test/ui/lub-glb/empty-binders.rs @@ -0,0 +1,45 @@ +// check-pass +// +// Check that computing the lub works even for empty binders. +fn lt<'a: 'a>() -> &'a () { + &() +} + +fn lt_in_fn<'a: 'a>() -> fn(&'a ()) { + |_| () +} + +struct Contra<'a>(fn(&'a ())); +fn lt_in_contra<'a: 'a>() -> Contra<'a> { + Contra(|_| ()) +} + +fn ok<'a, 'b, 'upper, 'lower>(v: bool) +where + 'upper: 'a, + 'upper: 'b, + 'a: 'lower, + 'b: 'lower, + +{ + let _: &'lower () = match v { + true => lt::<'a>(), + false => lt::<'b>(), + }; + + // This errored in the past because LUB and GLB always + // bailed out when encountering binders, even if they were + // empty. + let _: fn(&'upper ()) = match v { + true => lt_in_fn::<'a>(), + false => lt_in_fn::<'b>(), + }; + + // This was already accepted, as relate didn't encounter any binders. + let _: Contra<'upper> = match v { + true => lt_in_contra::<'a>(), + false => lt_in_contra::<'b>(), + }; +} + +fn main() {} From 0667b00acf939634ff1fd310ed5de2d49c65c6bb Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 8 Jun 2022 21:03:52 +0200 Subject: [PATCH 02/13] update tests + add future compat test --- .../ui/lub-glb/empty-binder-future-compat.rs | 22 +++++++++++++++++++ src/test/ui/lub-glb/empty-binders-err.rs | 12 +++------- src/test/ui/lub-glb/empty-binders-err.stderr | 20 ++++++++--------- 3 files changed, 35 insertions(+), 19 deletions(-) create mode 100644 src/test/ui/lub-glb/empty-binder-future-compat.rs diff --git a/src/test/ui/lub-glb/empty-binder-future-compat.rs b/src/test/ui/lub-glb/empty-binder-future-compat.rs new file mode 100644 index 0000000000000..8700a88a36ea0 --- /dev/null +++ b/src/test/ui/lub-glb/empty-binder-future-compat.rs @@ -0,0 +1,22 @@ +// check-pass +fn lt_in_fn_fn<'a: 'a>() -> fn(fn(&'a ())) { + |_| () +} + + +fn foo<'a, 'b, 'lower>(v: bool) +where + 'a: 'lower, + 'b: 'lower, +{ + // if we infer `x` to be higher ranked in the future, + // this would cause a type error. + let x = match v { + true => lt_in_fn_fn::<'a>(), + false => lt_in_fn_fn::<'b>(), + }; + + let _: fn(fn(&'lower())) = x; +} + +fn main() {} diff --git a/src/test/ui/lub-glb/empty-binders-err.rs b/src/test/ui/lub-glb/empty-binders-err.rs index ee28dd7c97d61..557480173ee62 100644 --- a/src/test/ui/lub-glb/empty-binders-err.rs +++ b/src/test/ui/lub-glb/empty-binders-err.rs @@ -11,12 +11,10 @@ fn lt_in_contra<'a: 'a>() -> Contra<'a> { Contra(|_| ()) } -fn covariance<'a, 'b, 'upper, 'lower>(v: bool) +fn covariance<'a, 'b, 'upper>(v: bool) where 'upper: 'a, 'upper: 'b, - 'a: 'lower, - 'b: 'lower, { let _: &'upper () = match v { @@ -27,10 +25,8 @@ where }; } -fn contra_fn<'a, 'b, 'upper, 'lower>(v: bool) +fn contra_fn<'a, 'b, 'lower>(v: bool) where - 'upper: 'a, - 'upper: 'b, 'a: 'lower, 'b: 'lower, @@ -43,10 +39,8 @@ where }; } -fn contra_struct<'a, 'b, 'upper, 'lower>(v: bool) +fn contra_struct<'a, 'b, 'lower>(v: bool) where - 'upper: 'a, - 'upper: 'b, 'a: 'lower, 'b: 'lower, diff --git a/src/test/ui/lub-glb/empty-binders-err.stderr b/src/test/ui/lub-glb/empty-binders-err.stderr index 0d5de978e4301..f86f22d5e40bf 100644 --- a/src/test/ui/lub-glb/empty-binders-err.stderr +++ b/src/test/ui/lub-glb/empty-binders-err.stderr @@ -1,7 +1,7 @@ error: lifetime may not live long enough - --> $DIR/empty-binders-err.rs:22:12 + --> $DIR/empty-binders-err.rs:20:12 | -LL | fn covariance<'a, 'b, 'upper, 'lower>(v: bool) +LL | fn covariance<'a, 'b, 'upper>(v: bool) | -- ------ lifetime `'upper` defined here | | | lifetime `'a` defined here @@ -12,9 +12,9 @@ LL | let _: &'upper () = match v { = help: consider adding the following bound: `'a: 'upper` error: lifetime may not live long enough - --> $DIR/empty-binders-err.rs:22:12 + --> $DIR/empty-binders-err.rs:20:12 | -LL | fn covariance<'a, 'b, 'upper, 'lower>(v: bool) +LL | fn covariance<'a, 'b, 'upper>(v: bool) | -- ------ lifetime `'upper` defined here | | | lifetime `'b` defined here @@ -30,10 +30,10 @@ help: the following changes may resolve your lifetime errors = help: add bound `'b: 'upper` error: lifetime may not live long enough - --> $DIR/empty-binders-err.rs:39:12 + --> $DIR/empty-binders-err.rs:35:12 | -LL | fn contra_fn<'a, 'b, 'upper, 'lower>(v: bool) - | -- ------ lifetime `'lower` defined here +LL | fn contra_fn<'a, 'b, 'lower>(v: bool) + | -- ------ lifetime `'lower` defined here | | | lifetime `'a` defined here ... @@ -43,10 +43,10 @@ LL | let _: fn(&'lower ()) = match v { = help: consider adding the following bound: `'lower: 'a` error: lifetime may not live long enough - --> $DIR/empty-binders-err.rs:54:12 + --> $DIR/empty-binders-err.rs:48:12 | -LL | fn contra_struct<'a, 'b, 'upper, 'lower>(v: bool) - | -- ------ lifetime `'lower` defined here +LL | fn contra_struct<'a, 'b, 'lower>(v: bool) + | -- ------ lifetime `'lower` defined here | | | lifetime `'a` defined here ... From 3f12fa7fda96de6687cdd281affcee4a61c35b80 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 26 Nov 2021 21:20:24 +0100 Subject: [PATCH 03/13] Add support for macro in "jump to def" feature --- src/librustdoc/html/format.rs | 1 + src/librustdoc/html/highlight.rs | 126 +++++++++++++++++++++---- src/librustdoc/html/render/span_map.rs | 71 ++++++++++---- 3 files changed, 162 insertions(+), 36 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 5baa53d55545f..29a58810036c6 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -519,6 +519,7 @@ impl clean::GenericArgs { } // Possible errors when computing href link source for a `DefId` +#[derive(PartialEq, Eq)] pub(crate) enum HrefError { /// This item is known to rustdoc, but from a crate that does not have documentation generated. /// diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 480728b179790..209172bb98e67 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,15 +5,19 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. -use crate::clean::PrimitiveType; +use crate::clean::{ExternalLocation, PrimitiveType}; use crate::html::escape::Escape; use crate::html::render::Context; use std::collections::VecDeque; use std::fmt::{Display, Write}; +use std::iter::once; +use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def_id::DefId; use rustc_lexer::{LiteralKind, TokenKind}; +use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use rustc_span::{BytePos, Span, DUMMY_SP}; @@ -99,6 +103,7 @@ fn write_code( ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); + let mut closing_tag = ""; Classifier::new( &src, edition, @@ -108,8 +113,8 @@ fn write_code( .highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => string(out, Escape(text), class, &context_info), - Highlight::EnterSpan { class } => enter_span(out, class), - Highlight::ExitSpan => exit_span(out), + Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &context_info), + Highlight::ExitSpan => exit_span(out, &closing_tag), }; }); } @@ -129,7 +134,7 @@ enum Class { RefKeyWord, Self_(Span), Op, - Macro, + Macro(Span), MacroNonTerminal, String, Number, @@ -153,7 +158,7 @@ impl Class { Class::RefKeyWord => "kw-2", Class::Self_(_) => "self", Class::Op => "op", - Class::Macro => "macro", + Class::Macro(_) => "macro", Class::MacroNonTerminal => "macro-nonterminal", Class::String => "string", Class::Number => "number", @@ -171,8 +176,22 @@ impl Class { /// a "span" (a tuple representing `(lo, hi)` equivalent of `Span`). fn get_span(self) -> Option { match self { - Self::Ident(sp) | Self::Self_(sp) => Some(sp), - _ => None, + Self::Ident(sp) | Self::Self_(sp) | Self::Macro(sp) => Some(sp), + Self::Comment + | Self::DocComment + | Self::Attribute + | Self::KeyWord + | Self::RefKeyWord + | Self::Op + | Self::MacroNonTerminal + | Self::String + | Self::Number + | Self::Bool + | Self::Lifetime + | Self::PreludeTy + | Self::PreludeVal + | Self::QuestionMark + | Self::Decoration(_) => None, } } } @@ -611,7 +630,7 @@ impl<'a> Classifier<'a> { }, TokenKind::Ident | TokenKind::RawIdent if lookahead == Some(TokenKind::Bang) => { self.in_macro = true; - sink(Highlight::EnterSpan { class: Class::Macro }); + sink(Highlight::EnterSpan { class: Class::Macro(self.new_span(before, text)) }); sink(Highlight::Token { text, class: None }); return; } @@ -658,13 +677,18 @@ impl<'a> Classifier<'a> { /// Called when we start processing a span of text that should be highlighted. /// The `Class` argument specifies how it should be highlighted. -fn enter_span(out: &mut Buffer, klass: Class) { - write!(out, "", klass.as_html()); +fn enter_span( + out: &mut Buffer, + klass: Class, + context_info: &Option>, +) -> &'static str { + string_without_closing_tag(out, "", Some(klass), context_info) + .expect("no closing tag to close wrapper...") } /// Called at the end of a span of highlighted text. -fn exit_span(out: &mut Buffer) { - out.write_str(""); +fn exit_span(out: &mut Buffer, closing_tag: &str) { + out.write_str(closing_tag); } /// Called for a span of text. If the text should be highlighted differently @@ -689,13 +713,28 @@ fn string( klass: Option, context_info: &Option>, ) { + if let Some(closing_tag) = string_without_closing_tag(out, text, klass, context_info) { + out.write_str(closing_tag); + } +} + +fn string_without_closing_tag( + out: &mut Buffer, + text: T, + klass: Option, + context_info: &Option>, +) -> Option<&'static str> { let Some(klass) = klass - else { return write!(out, "{}", text) }; + else { + write!(out, "{}", text); + return None; + }; let Some(def_span) = klass.get_span() else { - write!(out, "{}", klass.as_html(), text); - return; + write!(out, "{}", klass.as_html(), text); + return Some(""); }; + let mut text_s = text.to_string(); if text_s.contains("::") { text_s = text_s.split("::").intersperse("::").fold(String::new(), |mut path, t| { @@ -730,8 +769,17 @@ fn string( .map(|s| format!("{}{}", context_info.root_path, s)), LinkFromSrc::External(def_id) => { format::href_with_root_path(*def_id, context, Some(context_info.root_path)) - .ok() .map(|(url, _, _)| url) + .or_else(|e| { + if e == format::HrefError::NotInExternalCache + && matches!(klass, Class::Macro(_)) + { + Ok(generate_macro_def_id_path(context_info, *def_id)) + } else { + Err(e) + } + }) + .ok() } LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], @@ -743,11 +791,51 @@ fn string( } }) { - write!(out, "{}", klass.as_html(), href, text_s); - return; + write!(out, "{}", klass.as_html(), href, text_s); + return Some(""); } } - write!(out, "{}", klass.as_html(), text_s); + write!(out, "{}", klass.as_html(), text_s); + Some("") +} + +/// This function is to get the external macro path because they are not in the cache used n +/// `href_with_root_path`. +fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: DefId) -> String { + let tcx = context_info.context.shared.tcx; + let crate_name = tcx.crate_name(def_id.krate).to_string(); + let cache = &context_info.context.cache(); + + let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { + // extern blocks have an empty name + let s = elem.data.to_string(); + if !s.is_empty() { Some(s) } else { None } + }); + // Check to see if it is a macro 2.0 or built-in macro + let mut path = if matches!( + CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess), + LoadedMacro::MacroDef(def, _) + if matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) + if !ast_def.macro_rules) + ) { + once(crate_name.clone()).chain(relative).collect() + } else { + vec![crate_name.clone(), relative.last().expect("relative was empty")] + }; + + let url_parts = match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], + ExternalLocation::Local => vec![context_info.root_path.trim_end_matches('/'), &crate_name], + ExternalLocation::Unknown => panic!("unknown crate"), + }; + + let last = path.pop().unwrap(); + let last = format!("macro.{}.html", last); + if path.is_empty() { + format!("{}/{}", url_parts.join("/"), last) + } else { + format!("{}/{}/{}", url_parts.join("/"), path.join("/"), last) + } } #[cfg(test)] diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 86961dc3bf149..0c60278a82dd9 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -8,7 +8,8 @@ use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{ExprKind, HirId, Mod, Node}; use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; -use rustc_span::Span; +use rustc_span::hygiene::MacroKind; +use rustc_span::{BytePos, ExpnKind, Span}; use std::path::{Path, PathBuf}; @@ -63,32 +64,59 @@ struct SpanMapVisitor<'tcx> { impl<'tcx> SpanMapVisitor<'tcx> { /// This function is where we handle `hir::Path` elements and add them into the "span map". - fn handle_path(&mut self, path: &rustc_hir::Path<'_>, path_span: Option) { + fn handle_path(&mut self, path: &rustc_hir::Path<'_>) { let info = match path.res { - // FIXME: For now, we only handle `DefKind` if it's not `DefKind::TyParam` or - // `DefKind::Macro`. Would be nice to support them too alongside the other `DefKind` + // FIXME: For now, we handle `DefKind` if it's not a `DefKind::TyParam`. + // Would be nice to support them too alongside the other `DefKind` // (such as primitive types!). - Res::Def(kind, def_id) if kind != DefKind::TyParam => { - if matches!(kind, DefKind::Macro(_)) { - return; - } - Some(def_id) - } + Res::Def(kind, def_id) if kind != DefKind::TyParam => Some(def_id), Res::Local(_) => None, Res::PrimTy(p) => { // FIXME: Doesn't handle "path-like" primitives like arrays or tuples. - let span = path_span.unwrap_or(path.span); - self.matches.insert(span, LinkFromSrc::Primitive(PrimitiveType::from(p))); + self.matches.insert(path.span, LinkFromSrc::Primitive(PrimitiveType::from(p))); return; } Res::Err => return, _ => return, }; if let Some(span) = self.tcx.hir().res_span(path.res) { - self.matches - .insert(path_span.unwrap_or(path.span), LinkFromSrc::Local(clean::Span::new(span))); + self.matches.insert(path.span, LinkFromSrc::Local(clean::Span::new(span))); } else if let Some(def_id) = info { - self.matches.insert(path_span.unwrap_or(path.span), LinkFromSrc::External(def_id)); + self.matches.insert(path.span, LinkFromSrc::External(def_id)); + } + } + + /// Adds the macro call into the span map. Returns `true` if the `span` was inside a macro + /// expansion, whether or not it was added to the span map. + fn handle_macro(&mut self, span: Span) -> bool { + if span.from_expansion() { + let mut data = span.ctxt().outer_expn_data(); + let mut call_site = data.call_site; + while call_site.from_expansion() { + data = call_site.ctxt().outer_expn_data(); + call_site = data.call_site; + } + + if let ExpnKind::Macro(MacroKind::Bang, macro_name) = data.kind { + let link_from_src = if let Some(macro_def_id) = data.macro_def_id { + if macro_def_id.is_local() { + LinkFromSrc::Local(clean::Span::new(data.def_site)) + } else { + LinkFromSrc::External(macro_def_id) + } + } else { + return true; + }; + let new_span = data.call_site; + let macro_name = macro_name.as_str(); + // The "call_site" includes the whole macro with its "arguments". We only want + // the macro name. + let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); + self.matches.insert(new_span, link_from_src); + } + true + } else { + false } } } @@ -101,7 +129,10 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } fn visit_path(&mut self, path: &'tcx rustc_hir::Path<'tcx>, _id: HirId) { - self.handle_path(path, None); + if self.handle_macro(path.span) { + return; + } + self.handle_path(path); intravisit::walk_path(self, path); } @@ -143,12 +174,18 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { ); } } + } else if self.handle_macro(expr.span) { + // We don't want to deeper into the macro. + return; } intravisit::walk_expr(self, expr); } fn visit_use(&mut self, path: &'tcx rustc_hir::Path<'tcx>, id: HirId) { - self.handle_path(path, None); + if self.handle_macro(path.span) { + return; + } + self.handle_path(path); intravisit::walk_use(self, path, id); } } From dda980dec07fb7093a153180f19f967ce55fe1f9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 18 Jun 2022 15:32:26 +0200 Subject: [PATCH 04/13] Rename ContextInfo into HrefContext --- src/librustdoc/html/highlight.rs | 50 ++++++++++++++++---------------- src/librustdoc/html/sources.rs | 2 +- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 209172bb98e67..4c54b569762c6 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -26,7 +26,7 @@ use super::format::{self, Buffer}; use super::render::LinkFromSrc; /// This type is needed in case we want to render links on items to allow to go to their definition. -pub(crate) struct ContextInfo<'a, 'b, 'c> { +pub(crate) struct HrefContext<'a, 'b, 'c> { pub(crate) context: &'a Context<'b>, /// This span contains the current file we're going through. pub(crate) file_span: Span, @@ -48,7 +48,7 @@ pub(crate) fn render_with_highlighting( tooltip: Option<(Option, &str)>, edition: Edition, extra_content: Option, - context_info: Option>, + href_context: Option>, decoration_info: Option, ) { debug!("highlighting: ================\n{}\n==============", src); @@ -66,7 +66,7 @@ pub(crate) fn render_with_highlighting( } write_header(out, class, extra_content); - write_code(out, src, edition, context_info, decoration_info); + write_code(out, src, edition, href_context, decoration_info); write_footer(out, playground_button); } @@ -89,8 +89,8 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option>, + href_context: Option>, decoration_info: Option, ) { // This replace allows to fix how the code source with DOS backline characters is displayed. @@ -107,13 +107,13 @@ fn write_code( Classifier::new( &src, edition, - context_info.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP), + href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP), decoration_info, ) .highlight(&mut |highlight| { match highlight { - Highlight::Token { text, class } => string(out, Escape(text), class, &context_info), - Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &context_info), + Highlight::Token { text, class } => string(out, Escape(text), class, &href_context), + Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &href_context), Highlight::ExitSpan => exit_span(out, &closing_tag), }; }); @@ -680,9 +680,9 @@ impl<'a> Classifier<'a> { fn enter_span( out: &mut Buffer, klass: Class, - context_info: &Option>, + href_context: &Option>, ) -> &'static str { - string_without_closing_tag(out, "", Some(klass), context_info) + string_without_closing_tag(out, "", Some(klass), href_context) .expect("no closing tag to close wrapper...") } @@ -711,9 +711,9 @@ fn string( out: &mut Buffer, text: T, klass: Option, - context_info: &Option>, + href_context: &Option>, ) { - if let Some(closing_tag) = string_without_closing_tag(out, text, klass, context_info) { + if let Some(closing_tag) = string_without_closing_tag(out, text, klass, href_context) { out.write_str(closing_tag); } } @@ -722,7 +722,7 @@ fn string_without_closing_tag( out: &mut Buffer, text: T, klass: Option, - context_info: &Option>, + href_context: &Option>, ) -> Option<&'static str> { let Some(klass) = klass else { @@ -754,10 +754,10 @@ fn string_without_closing_tag( path }); } - if let Some(context_info) = context_info { + if let Some(href_context) = href_context { if let Some(href) = - context_info.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { - let context = context_info.context; + href_context.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + let context = href_context.context; // FIXME: later on, it'd be nice to provide two links (if possible) for all items: // one to the documentation page and one to the source definition. // FIXME: currently, external items only generate a link to their documentation, @@ -766,15 +766,15 @@ fn string_without_closing_tag( match href { LinkFromSrc::Local(span) => context .href_from_span(*span, true) - .map(|s| format!("{}{}", context_info.root_path, s)), + .map(|s| format!("{}{}", href_context.root_path, s)), LinkFromSrc::External(def_id) => { - format::href_with_root_path(*def_id, context, Some(context_info.root_path)) + format::href_with_root_path(*def_id, context, Some(href_context.root_path)) .map(|(url, _, _)| url) .or_else(|e| { if e == format::HrefError::NotInExternalCache && matches!(klass, Class::Macro(_)) { - Ok(generate_macro_def_id_path(context_info, *def_id)) + Ok(generate_macro_def_id_path(href_context, *def_id)) } else { Err(e) } @@ -784,7 +784,7 @@ fn string_without_closing_tag( LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], context, - Some(context_info.root_path), + Some(href_context.root_path), ) .ok() .map(|(url, _, _)| url), @@ -801,10 +801,10 @@ fn string_without_closing_tag( /// This function is to get the external macro path because they are not in the cache used n /// `href_with_root_path`. -fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: DefId) -> String { - let tcx = context_info.context.shared.tcx; +fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { + let tcx = href_context.context.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = &context_info.context.cache(); + let cache = &href_context.context.cache(); let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { // extern blocks have an empty name @@ -825,7 +825,7 @@ fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: De let url_parts = match cache.extern_locations[&def_id.krate] { ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], - ExternalLocation::Local => vec![context_info.root_path.trim_end_matches('/'), &crate_name], + ExternalLocation::Local => vec![href_context.root_path.trim_end_matches('/'), &crate_name], ExternalLocation::Unknown => panic!("unknown crate"), }; diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 524c90e1f4d64..bc0a56f2b052f 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -299,7 +299,7 @@ pub(crate) fn print_src( None, edition, Some(line_numbers), - Some(highlight::ContextInfo { context, file_span, root_path }), + Some(highlight::HrefContext { context, file_span, root_path }), decoration_info, ); } From 810254b31e0135d1f2dfe9e3717c3c0f69676a86 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 18 Jun 2022 15:50:37 +0200 Subject: [PATCH 05/13] Improve code readability and documentation --- src/librustdoc/html/highlight.rs | 80 +++++++++++++++++--------- src/librustdoc/html/render/span_map.rs | 68 +++++++++++++--------- 2 files changed, 94 insertions(+), 54 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 4c54b569762c6..11ab3a3f931c3 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -103,7 +103,7 @@ fn write_code( ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - let mut closing_tag = ""; + let mut closing_tags: Vec<&'static str> = Vec::new(); Classifier::new( &src, edition, @@ -113,8 +113,12 @@ fn write_code( .highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => string(out, Escape(text), class, &href_context), - Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &href_context), - Highlight::ExitSpan => exit_span(out, &closing_tag), + Highlight::EnterSpan { class } => { + closing_tags.push(enter_span(out, class, &href_context)) + } + Highlight::ExitSpan => { + exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan")) + } }; }); } @@ -682,8 +686,10 @@ fn enter_span( klass: Class, href_context: &Option>, ) -> &'static str { - string_without_closing_tag(out, "", Some(klass), href_context) - .expect("no closing tag to close wrapper...") + string_without_closing_tag(out, "", Some(klass), href_context).expect( + "internal error: enter_span was called with Some(klass) but did not return a \ + closing HTML tag", + ) } /// Called at the end of a span of highlighted text. @@ -718,6 +724,15 @@ fn string( } } +/// This function writes `text` into `out` with some modifications depending on `klass`: +/// +/// * If `klass` is `None`, `text` is written into `out` with no modification. +/// * If `klass` is `Some` but `klass.get_span()` is `None`, it writes the text wrapped in a +/// `` with the provided `klass`. +/// * If `klass` is `Some` and has a [`rustc_span::Span`], it then tries to generate a link (`` +/// element) by retrieving the link information from the `span_correspondance_map` that was filled +/// in `span_map.rs::collect_spans_and_sources`. If it cannot retrieve the information, then it's +/// the same as the second point (`klass` is `Some` but doesn't have a [`rustc_span::Span`]). fn string_without_closing_tag( out: &mut Buffer, text: T, @@ -799,42 +814,55 @@ fn string_without_closing_tag( Some("") } -/// This function is to get the external macro path because they are not in the cache used n +/// This function is to get the external macro path because they are not in the cache used in /// `href_with_root_path`. fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { let tcx = href_context.context.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = &href_context.context.cache(); + let cache = href_context.context.cache(); let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { // extern blocks have an empty name let s = elem.data.to_string(); if !s.is_empty() { Some(s) } else { None } }); - // Check to see if it is a macro 2.0 or built-in macro - let mut path = if matches!( - CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess), - LoadedMacro::MacroDef(def, _) - if matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) - if !ast_def.macro_rules) - ) { + // Check to see if it is a macro 2.0 or built-in macro. + // More information in . + let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + LoadedMacro::MacroDef(def, _) => { + // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. + matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) + } + _ => false, + }; + + let mut path = if is_macro_2 { once(crate_name.clone()).chain(relative).collect() } else { - vec![crate_name.clone(), relative.last().expect("relative was empty")] + vec![crate_name.clone(), relative.last().unwrap()] }; + if path.len() < 2 { + // The minimum we can have is the crate name followed by the macro name. If shorter, then + // it means that that `relative` was empty, which is an error. + panic!("macro path cannot be empty!"); + } - let url_parts = match cache.extern_locations[&def_id.krate] { - ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], - ExternalLocation::Local => vec![href_context.root_path.trim_end_matches('/'), &crate_name], - ExternalLocation::Unknown => panic!("unknown crate"), - }; + if let Some(last) = path.last_mut() { + *last = format!("macro.{}.html", last); + } - let last = path.pop().unwrap(); - let last = format!("macro.{}.html", last); - if path.is_empty() { - format!("{}/{}", url_parts.join("/"), last) - } else { - format!("{}/{}/{}", url_parts.join("/"), path.join("/"), last) + match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => { + // `ExternalLocation::Remote` always end with a `/`. + format!("{}{}", s, path.join("/")) + } + ExternalLocation::Local => { + // `href_context.root_path` always end with a `/`. + format!("{}{}/{}", href_context.root_path, crate_name, path.join("/")) + } + ExternalLocation::Unknown => { + panic!("crate {} not in cache when linkifying macros", crate_name) + } } } diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 0c60278a82dd9..34d590fb2448c 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -88,36 +88,48 @@ impl<'tcx> SpanMapVisitor<'tcx> { /// Adds the macro call into the span map. Returns `true` if the `span` was inside a macro /// expansion, whether or not it was added to the span map. + /// + /// The idea for the macro support is to check if the current `Span` comes from expansion. If + /// so, we loop until we find the macro definition by using `outer_expn_data` in a loop. + /// Finally, we get the information about the macro itself (`span` if "local", `DefId` + /// otherwise) and store it inside the span map. fn handle_macro(&mut self, span: Span) -> bool { - if span.from_expansion() { - let mut data = span.ctxt().outer_expn_data(); - let mut call_site = data.call_site; - while call_site.from_expansion() { - data = call_site.ctxt().outer_expn_data(); - call_site = data.call_site; - } + if !span.from_expansion() { + return false; + } + // So if the `span` comes from a macro expansion, we need to get the original + // macro's `DefId`. + let mut data = span.ctxt().outer_expn_data(); + let mut call_site = data.call_site; + // Macros can expand to code containing macros, which will in turn be expanded, etc. + // So the idea here is to "go up" until we're back to code that was generated from + // macro expansion so that we can get the `DefId` of the original macro that was at the + // origin of this expansion. + while call_site.from_expansion() { + data = call_site.ctxt().outer_expn_data(); + call_site = data.call_site; + } - if let ExpnKind::Macro(MacroKind::Bang, macro_name) = data.kind { - let link_from_src = if let Some(macro_def_id) = data.macro_def_id { - if macro_def_id.is_local() { - LinkFromSrc::Local(clean::Span::new(data.def_site)) - } else { - LinkFromSrc::External(macro_def_id) - } - } else { - return true; - }; - let new_span = data.call_site; - let macro_name = macro_name.as_str(); - // The "call_site" includes the whole macro with its "arguments". We only want - // the macro name. - let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); - self.matches.insert(new_span, link_from_src); + let macro_name = match data.kind { + ExpnKind::Macro(MacroKind::Bang, macro_name) => macro_name, + // Even though we don't handle this kind of macro, this `data` still comes from + // expansion so we return `true` so we don't go any deeper in this code. + _ => return true, + }; + let link_from_src = match data.macro_def_id { + Some(macro_def_id) if macro_def_id.is_local() => { + LinkFromSrc::Local(clean::Span::new(data.def_site)) } - true - } else { - false - } + Some(macro_def_id) => LinkFromSrc::External(macro_def_id), + None => return true, + }; + let new_span = data.call_site; + let macro_name = macro_name.as_str(); + // The "call_site" includes the whole macro with its "arguments". We only want + // the macro name. + let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); + self.matches.insert(new_span, link_from_src); + true } } @@ -175,7 +187,7 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } } } else if self.handle_macro(expr.span) { - // We don't want to deeper into the macro. + // We don't want to go deeper into the macro. return; } intravisit::walk_expr(self, expr); From f4db07ed4c1ab8a0f7961efc60ec32193e971f5c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 26 Nov 2021 21:20:45 +0100 Subject: [PATCH 06/13] Add test for macro support in "jump to def" feature --- .../check-source-code-urls-to-def-std.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) 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 b129ceb5b7302..3396b234a77b1 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 @@ -15,3 +15,28 @@ pub fn foo(a: u32, b: &str, c: String) { let y: bool = true; babar(); } + +macro_rules! yolo { () => {}} + +fn bar(a: i32) {} + +macro_rules! bar { + ($a:ident) => { bar($a) } +} + +macro_rules! data { + ($x:expr) => { $x * 2 } +} + +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!' + 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!' + let x = data!(4); + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#23-25"]' 'bar!' + bar!(x); +} From 987c73158e2120ef75b4b7fc46dcd88a621106d8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 22:32:49 +0200 Subject: [PATCH 07/13] Integrate `generate_macro_def_id_path` into `href_with_root_path` --- src/librustdoc/html/format.rs | 71 +++++++++++++++++++++++++++++++- src/librustdoc/html/highlight.rs | 69 +------------------------------ 2 files changed, 72 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 29a58810036c6..67b01245ef7ad 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -8,14 +8,16 @@ use std::borrow::Cow; use std::cell::Cell; use std::fmt; -use std::iter; +use std::iter::{self, once}; +use rustc_ast as ast; use rustc_attr::{ConstStability, StabilityLevel}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; +use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_middle::ty; use rustc_middle::ty::DefIdTree; use rustc_middle::ty::TyCtxt; @@ -557,6 +559,71 @@ pub(crate) fn join_with_double_colon(syms: &[Symbol]) -> String { s } +/// This function is to get the external macro path because they are not in the cache used in +/// `href_with_root_path`. +fn generate_macro_def_id_path( + def_id: DefId, + cx: &Context<'_>, + root_path: Option<&str>, +) -> (String, ItemType, Vec) { + let tcx = cx.shared.tcx; + let crate_name = tcx.crate_name(def_id.krate).to_string(); + let cache = cx.cache(); + + let fqp: Vec = tcx + .def_path(def_id) + .data + .into_iter() + .filter_map(|elem| { + // extern blocks (and a few others things) have an empty name. + match elem.data.get_opt_name() { + Some(s) if !s.is_empty() => Some(s), + _ => None, + } + }) + .collect(); + let relative = fqp.iter().map(|elem| elem.to_string()); + // Check to see if it is a macro 2.0 or built-in macro. + // More information in . + let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + LoadedMacro::MacroDef(def, _) => { + // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. + matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) + } + _ => false, + }; + + let mut path = if is_macro_2 { + once(crate_name.clone()).chain(relative).collect() + } else { + vec![crate_name.clone(), relative.last().unwrap()] + }; + if path.len() < 2 { + // The minimum we can have is the crate name followed by the macro name. If shorter, then + // it means that that `relative` was empty, which is an error. + panic!("macro path cannot be empty!"); + } + + if let Some(last) = path.last_mut() { + *last = format!("macro.{}.html", last); + } + + let url = match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => { + // `ExternalLocation::Remote` always end with a `/`. + format!("{}{}", s, path.join("/")) + } + ExternalLocation::Local => { + // `root_path` always end with a `/`. + format!("{}{}/{}", root_path.unwrap_or(""), crate_name, path.join("/")) + } + ExternalLocation::Unknown => { + panic!("crate {} not in cache when linkifying macros", crate_name) + } + }; + (url, ItemType::Macro, fqp) +} + pub(crate) fn href_with_root_path( did: DefId, cx: &Context<'_>, @@ -612,6 +679,8 @@ pub(crate) fn href_with_root_path( ExternalLocation::Unknown => return Err(HrefError::DocumentationNotBuilt), }, ) + } else if matches!(def_kind, DefKind::Macro(_)) { + return Ok(generate_macro_def_id_path(did, cx, root_path)); } else { return Err(HrefError::NotInExternalCache); } diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 11ab3a3f931c3..d2ef89078bf6d 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,19 +5,15 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. -use crate::clean::{ExternalLocation, PrimitiveType}; +use crate::clean::PrimitiveType; use crate::html::escape::Escape; use crate::html::render::Context; use std::collections::VecDeque; use std::fmt::{Display, Write}; -use std::iter::once; -use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::def_id::DefId; use rustc_lexer::{LiteralKind, TokenKind}; -use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use rustc_span::{BytePos, Span, DUMMY_SP}; @@ -784,17 +780,8 @@ fn string_without_closing_tag( .map(|s| format!("{}{}", href_context.root_path, s)), LinkFromSrc::External(def_id) => { format::href_with_root_path(*def_id, context, Some(href_context.root_path)) - .map(|(url, _, _)| url) - .or_else(|e| { - if e == format::HrefError::NotInExternalCache - && matches!(klass, Class::Macro(_)) - { - Ok(generate_macro_def_id_path(href_context, *def_id)) - } else { - Err(e) - } - }) .ok() + .map(|(url, _, _)| url) } LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], @@ -814,57 +801,5 @@ fn string_without_closing_tag( Some("") } -/// This function is to get the external macro path because they are not in the cache used in -/// `href_with_root_path`. -fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { - let tcx = href_context.context.shared.tcx; - let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = href_context.context.cache(); - - let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { - // extern blocks have an empty name - let s = elem.data.to_string(); - if !s.is_empty() { Some(s) } else { None } - }); - // Check to see if it is a macro 2.0 or built-in macro. - // More information in . - let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { - LoadedMacro::MacroDef(def, _) => { - // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. - matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) - } - _ => false, - }; - - let mut path = if is_macro_2 { - once(crate_name.clone()).chain(relative).collect() - } else { - vec![crate_name.clone(), relative.last().unwrap()] - }; - if path.len() < 2 { - // The minimum we can have is the crate name followed by the macro name. If shorter, then - // it means that that `relative` was empty, which is an error. - panic!("macro path cannot be empty!"); - } - - if let Some(last) = path.last_mut() { - *last = format!("macro.{}.html", last); - } - - match cache.extern_locations[&def_id.krate] { - ExternalLocation::Remote(ref s) => { - // `ExternalLocation::Remote` always end with a `/`. - format!("{}{}", s, path.join("/")) - } - ExternalLocation::Local => { - // `href_context.root_path` always end with a `/`. - format!("{}{}/{}", href_context.root_path, crate_name, path.join("/")) - } - ExternalLocation::Unknown => { - panic!("crate {} not in cache when linkifying macros", crate_name) - } - } -} - #[cfg(test)] mod tests; From eb6141ef6c665bab759f659179bb875a17bfe5e6 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Sat, 18 Jun 2022 20:00:21 -0700 Subject: [PATCH 08/13] Support setting file accessed/modified timestamps Add `struct FileTimes` to contain the relevant file timestamps, since most platforms require setting all of them at once. (This also allows for future platform-specific extensions such as setting creation time.) Add `File::set_file_time` to set the timestamps for a `File`. Implement the `sys` backends for UNIX, macOS (which needs to fall back to `futimes` before macOS 10.13 because it lacks `futimens`), Windows, and WASI. --- library/std/src/fs.rs | 72 +++++++++++++++++++++++++++ library/std/src/sys/unix/fs.rs | 64 ++++++++++++++++++++++++ library/std/src/sys/unsupported/fs.rs | 12 +++++ library/std/src/sys/wasi/fs.rs | 25 ++++++++++ library/std/src/sys/wasi/time.rs | 4 ++ library/std/src/sys/windows/c.rs | 8 ++- library/std/src/sys/windows/fs.rs | 24 +++++++++ library/std/src/sys/windows/time.rs | 7 +++ library/std/src/time.rs | 8 ++- 9 files changed, 222 insertions(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 55bd2c59406df..0500e826768af 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -184,6 +184,11 @@ pub struct DirEntry(fs_imp::DirEntry); #[stable(feature = "rust1", since = "1.0.0")] pub struct OpenOptions(fs_imp::OpenOptions); +/// Representation of the various timestamps on a file. +#[derive(Copy, Clone, Debug, Default)] +#[unstable(feature = "file_set_times", issue = "98245")] +pub struct FileTimes(fs_imp::FileTimes); + /// Representation of the various permissions on a file. /// /// This module only currently provides one bit of information, @@ -590,6 +595,49 @@ impl File { pub fn set_permissions(&self, perm: Permissions) -> io::Result<()> { self.inner.set_permissions(perm.0) } + + /// Changes the timestamps of the underlying file. + /// + /// # Platform-specific behavior + /// + /// This function currently corresponds to the `futimens` function on Unix (falling back to + /// `futimes` on macOS before 10.13) and the `SetFileTime` function on Windows. Note that this + /// [may change in the future][changes]. + /// + /// [changes]: io#platform-specific-behavior + /// + /// # Errors + /// + /// This function will return an error if the user lacks permission to change timestamps on the + /// underlying file. It may also return an error in other os-specific unspecified cases. + /// + /// This function may return an error if the operating system lacks support to change one or + /// more of the timestamps set in the `FileTimes` structure. + /// + /// # Examples + /// + /// ```no_run + /// #![feature(file_set_times)] + /// + /// fn main() -> std::io::Result<()> { + /// use std::fs::{self, File, FileTimes}; + /// + /// let src = fs::metadata("src")?; + /// let dest = File::options().write(true).open("dest")?; + /// let times = FileTimes::new() + /// .set_accessed(src.accessed()?) + /// .set_modified(src.modified()?); + /// dest.set_times(times)?; + /// Ok(()) + /// } + /// ``` + #[unstable(feature = "file_set_times", issue = "98245")] + #[doc(alias = "futimens")] + #[doc(alias = "futimes")] + #[doc(alias = "SetFileTime")] + pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + self.inner.set_times(times.0) + } } // In addition to the `impl`s here, `File` also has `impl`s for @@ -1246,6 +1294,30 @@ impl FromInner for Metadata { } } +impl FileTimes { + /// Create a new `FileTimes` with no times set. + /// + /// Using the resulting `FileTimes` in [`File::set_times`] will not modify any timestamps. + #[unstable(feature = "file_set_times", issue = "98245")] + pub fn new() -> Self { + Self::default() + } + + /// Set the last access time of a file. + #[unstable(feature = "file_set_times", issue = "98245")] + pub fn set_accessed(mut self, t: SystemTime) -> Self { + self.0.set_accessed(t.into_inner()); + self + } + + /// Set the last modified time of a file. + #[unstable(feature = "file_set_times", issue = "98245")] + pub fn set_modified(mut self, t: SystemTime) -> Self { + self.0.set_modified(t.into_inner()); + self + } +} + impl Permissions { /// Returns `true` if these permissions describe a readonly (unwritable) file. /// diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index 8b0bbd6a55c6b..5cb81d4c436d0 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -311,6 +311,9 @@ pub struct FilePermissions { mode: mode_t, } +#[derive(Copy, Clone)] +pub struct FileTimes([libc::timespec; 2]); + #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] pub struct FileType { mode: mode_t, @@ -503,6 +506,43 @@ impl FilePermissions { } } +impl FileTimes { + pub fn set_accessed(&mut self, t: SystemTime) { + self.0[0] = t.t.to_timespec().expect("Invalid system time"); + } + + pub fn set_modified(&mut self, t: SystemTime) { + self.0[1] = t.t.to_timespec().expect("Invalid system time"); + } +} + +struct TimespecDebugAdapter<'a>(&'a libc::timespec); + +impl fmt::Debug for TimespecDebugAdapter<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("timespec") + .field("tv_sec", &self.0.tv_sec) + .field("tv_nsec", &self.0.tv_nsec) + .finish() + } +} + +impl fmt::Debug for FileTimes { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FileTimes") + .field("accessed", &TimespecDebugAdapter(&self.0[0])) + .field("modified", &TimespecDebugAdapter(&self.0[1])) + .finish() + } +} + +impl Default for FileTimes { + fn default() -> Self { + let omit = libc::timespec { tv_sec: 0, tv_nsec: libc::UTIME_OMIT }; + Self([omit; 2]) + } +} + impl FileType { pub fn is_dir(&self) -> bool { self.is(libc::S_IFDIR) @@ -1021,6 +1061,30 @@ impl File { cvt_r(|| unsafe { libc::fchmod(self.as_raw_fd(), perm.mode) })?; Ok(()) } + + pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + cfg_if::cfg_if! { + // futimens requires macOS 10.13 + if #[cfg(target_os = "macos")] { + fn ts_to_tv(ts: &libc::timespec) -> libc::timeval { + libc::timeval { tv_sec: ts.tv_sec, tv_usec: ts.tv_nsec / 1000 } + } + cvt(unsafe { + let futimens = weak!(fn futimens(c_int, *const libc::timespec) -> c_int); + futimens.get() + .map(|futimens| futimens(self.as_raw_fd(), times.0.as_ptr())) + .unwrap_or_else(|| { + let timevals = [ts_to_tv(times.0[0]), ts_to_tv(times.0[1])]; + libc::futimes(self.as_raw_fd(), timevals.as_ptr()) + }) + })?; + } else { + cvt(unsafe { libc::futimens(self.as_raw_fd(), times.0.as_ptr()) })?; + } + } + + Ok(()) + } } impl DirBuilder { diff --git a/library/std/src/sys/unsupported/fs.rs b/library/std/src/sys/unsupported/fs.rs index d1d2847cd33be..0e1a6257ed763 100644 --- a/library/std/src/sys/unsupported/fs.rs +++ b/library/std/src/sys/unsupported/fs.rs @@ -17,6 +17,9 @@ pub struct DirEntry(!); #[derive(Clone, Debug)] pub struct OpenOptions {} +#[derive(Copy, Clone, Debug, Default)] +pub struct FileTimes {} + pub struct FilePermissions(!); pub struct FileType(!); @@ -86,6 +89,11 @@ impl fmt::Debug for FilePermissions { } } +impl FileTimes { + pub fn set_accessed(&mut self, _t: SystemTime) {} + pub fn set_modified(&mut self, _t: SystemTime) {} +} + impl FileType { pub fn is_dir(&self) -> bool { self.0 @@ -237,6 +245,10 @@ impl File { pub fn set_permissions(&self, _perm: FilePermissions) -> io::Result<()> { self.0 } + + pub fn set_times(&self, _times: FileTimes) -> io::Result<()> { + self.0 + } } impl DirBuilder { diff --git a/library/std/src/sys/wasi/fs.rs b/library/std/src/sys/wasi/fs.rs index cd6815bfc2136..b0cc748037862 100644 --- a/library/std/src/sys/wasi/fs.rs +++ b/library/std/src/sys/wasi/fs.rs @@ -63,6 +63,12 @@ pub struct FilePermissions { readonly: bool, } +#[derive(Copy, Clone, Debug, Default)] +pub struct FileTimes { + accessed: Option, + modified: Option, +} + #[derive(PartialEq, Eq, Hash, Debug, Copy, Clone)] pub struct FileType { bits: wasi::Filetype, @@ -112,6 +118,16 @@ impl FilePermissions { } } +impl FileTimes { + pub fn set_accessed(&mut self, t: SystemTime) { + self.accessed = t.to_wasi_timestamp_or_panic(); + } + + pub fn set_modified(&mut self, t: SystemTime) { + self.modified = t.to_wasi_timestamp_or_panic(); + } +} + impl FileType { pub fn is_dir(&self) -> bool { self.bits == wasi::FILETYPE_DIRECTORY @@ -459,6 +475,15 @@ impl File { unsupported() } + pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + self.fd.filestat_set_times( + times.accessed.unwrap_or(0), + times.modified.unwrap_or(0), + times.accessed.map_or(0, || wasi::FSTFLAGS_ATIM) + | times.modified.map_or(0, || wasi::FSTFLAGS_MTIM), + ) + } + pub fn read_link(&self, file: &Path) -> io::Result { read_link(&self.fd, file) } diff --git a/library/std/src/sys/wasi/time.rs b/library/std/src/sys/wasi/time.rs index 088585654b948..3d326e49106ca 100644 --- a/library/std/src/sys/wasi/time.rs +++ b/library/std/src/sys/wasi/time.rs @@ -47,6 +47,10 @@ impl SystemTime { SystemTime(Duration::from_nanos(ts)) } + pub fn to_wasi_timestamp_or_panic(&self) -> wasi::Timestamp { + self.0.as_nanos().try_into().expect("time does not fit in WASI timestamp") + } + pub fn sub_time(&self, other: &SystemTime) -> Result { self.0.checked_sub(other.0).ok_or_else(|| other.0 - self.0) } diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 5469487df1eed..abb471213d251 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -607,7 +607,7 @@ pub struct SOCKADDR { } #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug, Default)] pub struct FILETIME { pub dwLowDateTime: DWORD, pub dwHighDateTime: DWORD, @@ -875,6 +875,12 @@ extern "system" { pub fn GetSystemDirectoryW(lpBuffer: LPWSTR, uSize: UINT) -> UINT; pub fn RemoveDirectoryW(lpPathName: LPCWSTR) -> BOOL; pub fn SetFileAttributesW(lpFileName: LPCWSTR, dwFileAttributes: DWORD) -> BOOL; + pub fn SetFileTime( + hFile: BorrowedHandle<'_>, + lpCreationTime: Option<&FILETIME>, + lpLastAccessTime: Option<&FILETIME>, + lpLastWriteTime: Option<&FILETIME>, + ) -> BOOL; pub fn SetLastError(dwErrCode: DWORD); pub fn GetCommandLineW() -> LPWSTR; pub fn GetTempPathW(nBufferLength: DWORD, lpBuffer: LPCWSTR) -> DWORD; diff --git a/library/std/src/sys/windows/fs.rs b/library/std/src/sys/windows/fs.rs index 157d8a044d3cd..e3809f3579dfd 100644 --- a/library/std/src/sys/windows/fs.rs +++ b/library/std/src/sys/windows/fs.rs @@ -82,6 +82,12 @@ pub struct FilePermissions { attrs: c::DWORD, } +#[derive(Copy, Clone, Debug, Default)] +pub struct FileTimes { + accessed: c::FILETIME, + modified: c::FILETIME, +} + #[derive(Debug)] pub struct DirBuilder; @@ -550,6 +556,14 @@ impl File { })?; Ok(()) } + + pub fn set_times(&self, times: FileTimes) -> io::Result<()> { + cvt(unsafe { + c::SetFileTime(self.as_handle(), None, Some(×.accessed), Some(×.modified)) + })?; + Ok(()) + } + /// Get only basic file information such as attributes and file times. fn basic_info(&self) -> io::Result { unsafe { @@ -895,6 +909,16 @@ impl FilePermissions { } } +impl FileTimes { + pub fn set_accessed(&mut self, t: SystemTime) { + self.accessed = t.into_inner(); + } + + pub fn set_modified(&mut self, t: SystemTime) { + self.modified = t.into_inner(); + } +} + impl FileType { fn new(attrs: c::DWORD, reparse_tag: c::DWORD) -> FileType { FileType { attributes: attrs, reparse_tag } diff --git a/library/std/src/sys/windows/time.rs b/library/std/src/sys/windows/time.rs index 8f46781c75340..b8209a8544585 100644 --- a/library/std/src/sys/windows/time.rs +++ b/library/std/src/sys/windows/time.rs @@ -2,6 +2,7 @@ use crate::cmp::Ordering; use crate::fmt; use crate::mem; use crate::sys::c; +use crate::sys_common::IntoInner; use crate::time::Duration; use core::hash::{Hash, Hasher}; @@ -136,6 +137,12 @@ impl From for SystemTime { } } +impl IntoInner for SystemTime { + fn into_inner(self) -> c::FILETIME { + self.t + } +} + impl Hash for SystemTime { fn hash(&self, state: &mut H) { self.intervals().hash(state) diff --git a/library/std/src/time.rs b/library/std/src/time.rs index b2014f462bd34..759a59e1f98d2 100644 --- a/library/std/src/time.rs +++ b/library/std/src/time.rs @@ -38,7 +38,7 @@ use crate::error::Error; use crate::fmt; use crate::ops::{Add, AddAssign, Sub, SubAssign}; use crate::sys::time; -use crate::sys_common::FromInner; +use crate::sys_common::{FromInner, IntoInner}; #[stable(feature = "time", since = "1.3.0")] pub use core::time::Duration; @@ -686,3 +686,9 @@ impl FromInner for SystemTime { SystemTime(time) } } + +impl IntoInner for SystemTime { + fn into_inner(self) -> time::SystemTime { + self.0 + } +} From 585767d8185a2f34e429a18b3ae9a75f11d13470 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Mon, 20 Jun 2022 14:37:38 -0700 Subject: [PATCH 09/13] Add alias `File::set_modified` as shorthand --- library/std/src/fs.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 0500e826768af..4f321cdeeea00 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -638,6 +638,15 @@ impl File { pub fn set_times(&self, times: FileTimes) -> io::Result<()> { self.inner.set_times(times.0) } + + /// Changes the modification time of the underlying file. + /// + /// This is an alias for `set_times(FileTimes::new().set_modified(time))`. + #[unstable(feature = "file_set_times", issue = "98245")] + #[inline] + pub fn set_modified(&self, time: SystemTime) -> io::Result<()> { + self.set_times(FileTimes::new().set_modified(time)) + } } // In addition to the `impl`s here, `File` also has `impl`s for From beb2f364cc85b4408da1d043f875d159003558e4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 23:31:40 +0200 Subject: [PATCH 10/13] Fix panic by checking if `CStore` has the crate data we want before actually querying it --- compiler/rustc_metadata/src/creader.rs | 4 ++++ src/librustdoc/html/format.rs | 20 ++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 947d563ae3cd1..555db5846edd0 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -133,6 +133,10 @@ impl CStore { CrateNum::new(self.metas.len() - 1) } + pub fn has_crate_data(&self, cnum: CrateNum) -> bool { + self.metas[cnum].is_some() + } + pub(crate) fn get_crate_data(&self, cnum: CrateNum) -> CrateMetadataRef<'_> { let cdata = self.metas[cnum] .as_ref() diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 67b01245ef7ad..056eda089c1de 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -565,7 +565,7 @@ fn generate_macro_def_id_path( def_id: DefId, cx: &Context<'_>, root_path: Option<&str>, -) -> (String, ItemType, Vec) { +) -> Result<(String, ItemType, Vec), HrefError> { let tcx = cx.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); let cache = cx.cache(); @@ -583,9 +583,15 @@ fn generate_macro_def_id_path( }) .collect(); let relative = fqp.iter().map(|elem| elem.to_string()); + let cstore = CStore::from_tcx(tcx); + // We need this to prevent a `panic` when this function is used from intra doc links... + if !cstore.has_crate_data(def_id.krate) { + debug!("No data for crate {}", crate_name); + return Err(HrefError::NotInExternalCache); + } // Check to see if it is a macro 2.0 or built-in macro. // More information in . - let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + let is_macro_2 = match cstore.load_macro_untracked(def_id, tcx.sess) { LoadedMacro::MacroDef(def, _) => { // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) @@ -601,7 +607,8 @@ fn generate_macro_def_id_path( if path.len() < 2 { // The minimum we can have is the crate name followed by the macro name. If shorter, then // it means that that `relative` was empty, which is an error. - panic!("macro path cannot be empty!"); + debug!("macro path cannot be empty!"); + return Err(HrefError::NotInExternalCache); } if let Some(last) = path.last_mut() { @@ -618,10 +625,11 @@ fn generate_macro_def_id_path( format!("{}{}/{}", root_path.unwrap_or(""), crate_name, path.join("/")) } ExternalLocation::Unknown => { - panic!("crate {} not in cache when linkifying macros", crate_name) + debug!("crate {} not in cache when linkifying macros", crate_name); + return Err(HrefError::NotInExternalCache); } }; - (url, ItemType::Macro, fqp) + Ok((url, ItemType::Macro, fqp)) } pub(crate) fn href_with_root_path( @@ -680,7 +688,7 @@ pub(crate) fn href_with_root_path( }, ) } else if matches!(def_kind, DefKind::Macro(_)) { - return Ok(generate_macro_def_id_path(did, cx, root_path)); + return generate_macro_def_id_path(did, cx, root_path); } else { return Err(HrefError::NotInExternalCache); } From e900a3549624860a466c33c04742ccfa026bdc6d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 21 Jun 2022 03:47:25 +0000 Subject: [PATCH 11/13] Give name if anonymous region appears in impl signature --- .../src/diagnostics/outlives_suggestion.rs | 3 +- .../src/diagnostics/region_name.rs | 58 +++++++++++++++++-- src/test/ui/nll/issue-98170.rs | 25 ++++++++ src/test/ui/nll/issue-98170.stderr | 44 ++++++++++++++ 4 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/nll/issue-98170.rs create mode 100644 src/test/ui/nll/issue-98170.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs b/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs index 9d81330745fe2..d359d7efb6268 100644 --- a/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs +++ b/compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs @@ -62,7 +62,8 @@ impl OutlivesSuggestionBuilder { | RegionNameSource::AnonRegionFromUpvar(..) | RegionNameSource::AnonRegionFromOutput(..) | RegionNameSource::AnonRegionFromYieldTy(..) - | RegionNameSource::AnonRegionFromAsyncFn(..) => { + | RegionNameSource::AnonRegionFromAsyncFn(..) + | RegionNameSource::AnonRegionFromImplSignature(..) => { debug!("Region {:?} is NOT suggestable", name); false } diff --git a/compiler/rustc_borrowck/src/diagnostics/region_name.rs b/compiler/rustc_borrowck/src/diagnostics/region_name.rs index d6b5089712ab4..b5ae85fd042c2 100644 --- a/compiler/rustc_borrowck/src/diagnostics/region_name.rs +++ b/compiler/rustc_borrowck/src/diagnostics/region_name.rs @@ -6,7 +6,7 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_middle::ty::print::RegionHighlightMode; use rustc_middle::ty::subst::{GenericArgKind, SubstsRef}; -use rustc_middle::ty::{self, RegionVid, Ty}; +use rustc_middle::ty::{self, DefIdTree, RegionVid, Ty}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; @@ -45,6 +45,8 @@ pub(crate) enum RegionNameSource { AnonRegionFromYieldTy(Span, String), /// An anonymous region from an async fn. AnonRegionFromAsyncFn(Span), + /// An anonymous region from an impl self type or trait + AnonRegionFromImplSignature(Span, &'static str), } /// Describes what to highlight to explain to the user that we're giving an anonymous region a @@ -75,7 +77,8 @@ impl RegionName { | RegionNameSource::AnonRegionFromUpvar(..) | RegionNameSource::AnonRegionFromOutput(..) | RegionNameSource::AnonRegionFromYieldTy(..) - | RegionNameSource::AnonRegionFromAsyncFn(..) => false, + | RegionNameSource::AnonRegionFromAsyncFn(..) + | RegionNameSource::AnonRegionFromImplSignature(..) => false, } } @@ -87,7 +90,8 @@ impl RegionName { | RegionNameSource::SynthesizedFreeEnvRegion(span, _) | RegionNameSource::AnonRegionFromUpvar(span, _) | RegionNameSource::AnonRegionFromYieldTy(span, _) - | RegionNameSource::AnonRegionFromAsyncFn(span) => Some(span), + | RegionNameSource::AnonRegionFromAsyncFn(span) + | RegionNameSource::AnonRegionFromImplSignature(span, _) => Some(span), RegionNameSource::AnonRegionFromArgument(ref highlight) | RegionNameSource::AnonRegionFromOutput(ref highlight, _) => match *highlight { RegionNameHighlight::MatchedHirTy(span) @@ -166,6 +170,12 @@ impl RegionName { RegionNameSource::AnonRegionFromYieldTy(span, type_name) => { diag.span_label(*span, format!("yield type is {type_name}")); } + RegionNameSource::AnonRegionFromImplSignature(span, location) => { + diag.span_label( + *span, + format!("lifetime `{self}` appears in the `impl`'s {location}"), + ); + } RegionNameSource::Static => {} } } @@ -240,7 +250,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { .or_else(|| self.give_name_if_anonymous_region_appears_in_arguments(fr)) .or_else(|| self.give_name_if_anonymous_region_appears_in_upvars(fr)) .or_else(|| self.give_name_if_anonymous_region_appears_in_output(fr)) - .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr)); + .or_else(|| self.give_name_if_anonymous_region_appears_in_yield_ty(fr)) + .or_else(|| self.give_name_if_anonymous_region_appears_in_impl_signature(fr)); if let Some(ref value) = value { self.region_names.try_borrow_mut().unwrap().insert(fr, value.clone()); @@ -840,4 +851,43 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { source: RegionNameSource::AnonRegionFromYieldTy(yield_span, type_name), }) } + + fn give_name_if_anonymous_region_appears_in_impl_signature( + &self, + fr: RegionVid, + ) -> Option { + let ty::ReEarlyBound(region) = *self.to_error_region(fr)? else { + return None; + }; + if region.has_name() { + return None; + }; + + let tcx = self.infcx.tcx; + let body_parent_did = tcx.opt_parent(self.mir_def_id().to_def_id())?; + if tcx.parent(region.def_id) != body_parent_did + || tcx.def_kind(body_parent_did) != DefKind::Impl + { + return None; + } + + let mut found = false; + tcx.fold_regions(tcx.type_of(body_parent_did), &mut true, |r: ty::Region<'tcx>, _| { + if *r == ty::ReEarlyBound(region) { + found = true; + } + r + }); + + Some(RegionName { + name: self.synthesize_region_name(), + source: RegionNameSource::AnonRegionFromImplSignature( + tcx.def_span(region.def_id), + // FIXME(compiler-errors): Does this ever actually show up + // anywhere other than the self type? I couldn't create an + // example of a `'_` in the impl's trait being referenceable. + if found { "self type" } else { "header" }, + ), + }) + } } diff --git a/src/test/ui/nll/issue-98170.rs b/src/test/ui/nll/issue-98170.rs new file mode 100644 index 0000000000000..6bb12f52d3f3e --- /dev/null +++ b/src/test/ui/nll/issue-98170.rs @@ -0,0 +1,25 @@ +pub struct MyStruct<'a> { + field: &'a [u32], +} + +impl MyStruct<'_> { + pub fn new<'a>(field: &'a [u32]) -> MyStruct<'a> { + Self { field } + //~^ ERROR lifetime may not live long enough + //~| ERROR lifetime may not live long enough + } +} + +trait Trait<'a> { + fn new(field: &'a [u32]) -> MyStruct<'a>; +} + +impl<'a> Trait<'a> for MyStruct<'_> { + fn new(field: &'a [u32]) -> MyStruct<'a> { + Self { field } + //~^ ERROR lifetime may not live long enough + //~| ERROR lifetime may not live long enough + } +} + +fn main() {} diff --git a/src/test/ui/nll/issue-98170.stderr b/src/test/ui/nll/issue-98170.stderr new file mode 100644 index 0000000000000..0d17365e71b4a --- /dev/null +++ b/src/test/ui/nll/issue-98170.stderr @@ -0,0 +1,44 @@ +error: lifetime may not live long enough + --> $DIR/issue-98170.rs:7:9 + | +LL | impl MyStruct<'_> { + | -- lifetime `'1` appears in the `impl`'s self type +LL | pub fn new<'a>(field: &'a [u32]) -> MyStruct<'a> { + | -- lifetime `'a` defined here +LL | Self { field } + | ^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1` + +error: lifetime may not live long enough + --> $DIR/issue-98170.rs:7:16 + | +LL | impl MyStruct<'_> { + | -- lifetime `'1` appears in the `impl`'s self type +LL | pub fn new<'a>(field: &'a [u32]) -> MyStruct<'a> { + | -- lifetime `'a` defined here +LL | Self { field } + | ^^^^^ this usage requires that `'a` must outlive `'1` + +error: lifetime may not live long enough + --> $DIR/issue-98170.rs:19:9 + | +LL | impl<'a> Trait<'a> for MyStruct<'_> { + | -- -- lifetime `'1` appears in the `impl`'s self type + | | + | lifetime `'a` defined here +LL | fn new(field: &'a [u32]) -> MyStruct<'a> { +LL | Self { field } + | ^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'1` + +error: lifetime may not live long enough + --> $DIR/issue-98170.rs:19:16 + | +LL | impl<'a> Trait<'a> for MyStruct<'_> { + | -- -- lifetime `'1` appears in the `impl`'s self type + | | + | lifetime `'a` defined here +LL | fn new(field: &'a [u32]) -> MyStruct<'a> { +LL | Self { field } + | ^^^^^ this usage requires that `'a` must outlive `'1` + +error: aborting due to 4 previous errors + From 31476e70969fce766519682072da37a65995ccf7 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Tue, 21 Jun 2022 19:08:48 +0900 Subject: [PATCH 12/13] Add a full regression test for #73727 Signed-off-by: Yuki Okushi --- ...-static-reference-array-const-param.min.stderr} | 2 +- ...sue-73727-static-reference-array-const-param.rs | 14 ++++++++++++++ .../static-reference-array-const-param.rs | 6 ------ 3 files changed, 15 insertions(+), 7 deletions(-) rename src/test/ui/const-generics/{min_const_generics/static-reference-array-const-param.stderr => issues/issue-73727-static-reference-array-const-param.min.stderr} (84%) create mode 100644 src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.rs delete mode 100644 src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.rs diff --git a/src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.stderr b/src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.min.stderr similarity index 84% rename from src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.stderr rename to src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.min.stderr index f30693221a513..0a7db62472a9f 100644 --- a/src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.stderr +++ b/src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.min.stderr @@ -1,5 +1,5 @@ error: `&'static [u32]` is forbidden as the type of a const generic parameter - --> $DIR/static-reference-array-const-param.rs:1:15 + --> $DIR/issue-73727-static-reference-array-const-param.rs:9:15 | LL | fn a() {} | ^^^^^^^^^^^^^^ diff --git a/src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.rs b/src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.rs new file mode 100644 index 0000000000000..f0d604835cbb6 --- /dev/null +++ b/src/test/ui/const-generics/issues/issue-73727-static-reference-array-const-param.rs @@ -0,0 +1,14 @@ +// Regression test for #73727 + +// revisions: full min +//[full]check-pass + +#![cfg_attr(full, feature(adt_const_params))] +#![cfg_attr(full, allow(incomplete_features))] + +fn a() {} +//[min]~^ ERROR `&'static [u32]` is forbidden as the type of a const generic parameter + +fn main() { + a::<{&[]}>(); +} diff --git a/src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.rs b/src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.rs deleted file mode 100644 index 7518dc59e599c..0000000000000 --- a/src/test/ui/const-generics/min_const_generics/static-reference-array-const-param.rs +++ /dev/null @@ -1,6 +0,0 @@ -fn a() {} -//~^ ERROR `&'static [u32]` is forbidden as the type of a const generic parameter - -fn main() { - a::<{&[]}>(); -} From 5ed149504180e04ba81e308b07c2d33af1b7af49 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 21 Jun 2022 12:40:32 -0300 Subject: [PATCH 13/13] This comment is out dated and misleading Arms are about TAIT and RPIT, as the variants clearly show. --- compiler/rustc_resolve/src/late/lifetimes.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/rustc_resolve/src/late/lifetimes.rs b/compiler/rustc_resolve/src/late/lifetimes.rs index 447f4174c10d5..55574b8360739 100644 --- a/compiler/rustc_resolve/src/late/lifetimes.rs +++ b/compiler/rustc_resolve/src/late/lifetimes.rs @@ -846,8 +846,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { // the opaque_ty generics let opaque_ty = self.tcx.hir().item(item_id); let (generics, bounds) = match opaque_ty.kind { - // Named opaque `impl Trait` types are reached via `TyKind::Path`. - // This arm is for `impl Trait` in the types of statics, constants and locals. hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin: hir::OpaqueTyOrigin::TyAlias, .. @@ -866,7 +864,6 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> { return; } - // RPIT (return position impl trait) hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin: hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..), ref generics,