From 5837518e78bf71172c4c86f14afd545fa38aa9e2 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 13:26:04 -0700 Subject: [PATCH 1/6] `-Zunpretty=everybody_loops` only works after expansion --- src/librustc_session/config.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index 5bdd7b67723b8..939d5e8ef8358 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1954,9 +1954,11 @@ impl PpMode { use PpMode::*; use PpSourceMode::*; match *self { - PpmSource(PpmNormal | PpmEveryBodyLoops | PpmIdentified) => false, + PpmSource(PpmNormal | PpmIdentified) => false, - PpmSource(PpmExpanded | PpmExpandedIdentified | PpmExpandedHygiene) + PpmSource( + PpmExpanded | PpmEveryBodyLoops | PpmExpandedIdentified | PpmExpandedHygiene, + ) | PpmHir(_) | PpmHirTree(_) | PpmMir From d6848550393e9cebc734d5b8772b96c3a149a460 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 13:26:04 -0700 Subject: [PATCH 2/6] Helper method for whether `ast::Ty` contains `impl Trait` --- src/librustc_ast/ast.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index efcf95ec706b8..8ae2e81fc5523 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -25,6 +25,7 @@ pub use UnsafeSource::*; use crate::ptr::P; use crate::token::{self, DelimToken}; use crate::tokenstream::{DelimSpan, TokenStream, TokenTree}; +use crate::visit::{walk_ty, Visitor}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::sync::Lrc; @@ -1784,6 +1785,26 @@ pub struct Ty { pub span: Span, } +impl Ty { + /// Returns `true` if the kind of this type or any types contained within are `impl Trait`. + pub fn contains_impl_trait(&self) -> bool { + struct ContainsImplTrait(bool); + + impl<'ast> Visitor<'ast> for ContainsImplTrait { + fn visit_ty(&mut self, t: &'ast Ty) { + self.0 |= matches!(t.kind, TyKind::ImplTrait(..)); + if !self.0 { + walk_ty(self, t); + } + } + } + + let mut vis = ContainsImplTrait(false); + vis.visit_ty(self); + vis.0 + } +} + #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub struct BareFnTy { pub unsafety: Unsafe, From b5fdbbef4e03ab4fb88fcadbb8c370a1126419d9 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 13:26:04 -0700 Subject: [PATCH 3/6] Use correct names for things in `ReplaceBodyWithLoops` --- src/librustc_interface/util.rs | 130 +++++++++++---------------------- 1 file changed, 41 insertions(+), 89 deletions(-) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 924908e572487..4d33383071e16 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -592,97 +592,48 @@ pub fn build_output_filenames( // // [#34511]: https://github.com/rust-lang/rust/issues/34511#issuecomment-322340401 pub struct ReplaceBodyWithLoop<'a, 'b> { - within_static_or_const: bool, + ignore_item: bool, nested_blocks: Option>, resolver: &'a mut Resolver<'b>, } impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> { pub fn new(resolver: &'a mut Resolver<'b>) -> ReplaceBodyWithLoop<'a, 'b> { - ReplaceBodyWithLoop { within_static_or_const: false, nested_blocks: None, resolver } + ReplaceBodyWithLoop { ignore_item: false, nested_blocks: None, resolver } } - fn run R>(&mut self, is_const: bool, action: F) -> R { - let old_const = mem::replace(&mut self.within_static_or_const, is_const); + fn run R>(&mut self, ignore_item: bool, action: F) -> R { + let old_ignore_item = mem::replace(&mut self.ignore_item, ignore_item); let old_blocks = self.nested_blocks.take(); let ret = action(self); - self.within_static_or_const = old_const; + self.ignore_item = old_ignore_item; self.nested_blocks = old_blocks; ret } - fn should_ignore_fn(ret_ty: &ast::FnRetTy) -> bool { - if let ast::FnRetTy::Ty(ref ty) = ret_ty { - fn involves_impl_trait(ty: &ast::Ty) -> bool { - match ty.kind { - ast::TyKind::ImplTrait(..) => true, - ast::TyKind::Slice(ref subty) - | ast::TyKind::Array(ref subty, _) - | ast::TyKind::Ptr(ast::MutTy { ty: ref subty, .. }) - | ast::TyKind::Rptr(_, ast::MutTy { ty: ref subty, .. }) - | ast::TyKind::Paren(ref subty) => involves_impl_trait(subty), - ast::TyKind::Tup(ref tys) => any_involves_impl_trait(tys.iter()), - ast::TyKind::Path(_, ref path) => { - path.segments.iter().any(|seg| match seg.args.as_deref() { - None => false, - Some(&ast::GenericArgs::AngleBracketed(ref data)) => { - data.args.iter().any(|arg| match arg { - ast::AngleBracketedArg::Arg(arg) => match arg { - ast::GenericArg::Type(ty) => involves_impl_trait(ty), - ast::GenericArg::Lifetime(_) - | ast::GenericArg::Const(_) => false, - }, - ast::AngleBracketedArg::Constraint(c) => match c.kind { - ast::AssocTyConstraintKind::Bound { .. } => true, - ast::AssocTyConstraintKind::Equality { ref ty } => { - involves_impl_trait(ty) - } - }, - }) - } - Some(&ast::GenericArgs::Parenthesized(ref data)) => { - any_involves_impl_trait(data.inputs.iter()) - || ReplaceBodyWithLoop::should_ignore_fn(&data.output) - } - }) - } - _ => false, - } - } - - fn any_involves_impl_trait<'a, I: Iterator>>(mut it: I) -> bool { - it.any(|subty| involves_impl_trait(subty)) - } - - involves_impl_trait(ty) - } else { - false - } - } - - fn is_sig_const(sig: &ast::FnSig) -> bool { + fn should_ignore_fn(sig: &ast::FnSig) -> bool { matches!(sig.header.constness, ast::Const::Yes(_)) - || ReplaceBodyWithLoop::should_ignore_fn(&sig.decl.output) + || matches!(&sig.decl.output, ast::FnRetTy::Ty(ty) if ty.contains_impl_trait()) } } impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { fn visit_item_kind(&mut self, i: &mut ast::ItemKind) { - let is_const = match i { + let ignore_item = match i { ast::ItemKind::Static(..) | ast::ItemKind::Const(..) => true, - ast::ItemKind::Fn(_, ref sig, _, _) => Self::is_sig_const(sig), + ast::ItemKind::Fn(_, ref sig, _, _) => Self::should_ignore_fn(sig), _ => false, }; - self.run(is_const, |s| noop_visit_item_kind(i, s)) + self.run(ignore_item, |s| noop_visit_item_kind(i, s)) } fn flat_map_trait_item(&mut self, i: P) -> SmallVec<[P; 1]> { - let is_const = match i.kind { + let ignore_item = match i.kind { ast::AssocItemKind::Const(..) => true, - ast::AssocItemKind::Fn(_, ref sig, _, _) => Self::is_sig_const(sig), + ast::AssocItemKind::Fn(_, ref sig, _, _) => Self::should_ignore_fn(sig), _ => false, }; - self.run(is_const, |s| noop_flat_map_assoc_item(i, s)) + self.run(ignore_item, |s| noop_flat_map_assoc_item(i, s)) } fn flat_map_impl_item(&mut self, i: P) -> SmallVec<[P; 1]> { @@ -723,6 +674,11 @@ impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { } } + if self.ignore_item { + noop_visit_block(b, self); + return; + } + let empty_block = stmt_to_block(BlockCheckMode::Default, None, self.resolver); let loop_expr = P(ast::Expr { kind: ast::ExprKind::Loop(P(empty_block), None), @@ -738,39 +694,35 @@ impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { kind: ast::StmtKind::Expr(loop_expr), }; - if self.within_static_or_const { - noop_visit_block(b, self) - } else { - visit_clobber(b.deref_mut(), |b| { - let mut stmts = vec![]; - for s in b.stmts { - let old_blocks = self.nested_blocks.replace(vec![]); + visit_clobber(b.deref_mut(), |b| { + let mut stmts = vec![]; + for s in b.stmts { + let old_blocks = self.nested_blocks.replace(vec![]); - stmts.extend(self.flat_map_stmt(s).into_iter().filter(|s| s.is_item())); + stmts.extend(self.flat_map_stmt(s).into_iter().filter(|s| s.is_item())); - // we put a Some in there earlier with that replace(), so this is valid - let new_blocks = self.nested_blocks.take().unwrap(); - self.nested_blocks = old_blocks; - stmts.extend(new_blocks.into_iter().map(|b| block_to_stmt(b, self.resolver))); - } + // we put a Some in there earlier with that replace(), so this is valid + let new_blocks = self.nested_blocks.take().unwrap(); + self.nested_blocks = old_blocks; + stmts.extend(new_blocks.into_iter().map(|b| block_to_stmt(b, self.resolver))); + } - let mut new_block = ast::Block { stmts, ..b }; + let mut new_block = ast::Block { stmts, ..b }; - if let Some(old_blocks) = self.nested_blocks.as_mut() { - //push our fresh block onto the cache and yield an empty block with `loop {}` - if !new_block.stmts.is_empty() { - old_blocks.push(new_block); - } + if let Some(old_blocks) = self.nested_blocks.as_mut() { + //push our fresh block onto the cache and yield an empty block with `loop {}` + if !new_block.stmts.is_empty() { + old_blocks.push(new_block); + } - stmt_to_block(b.rules, Some(loop_stmt), &mut self.resolver) - } else { - //push `loop {}` onto the end of our fresh block and yield that - new_block.stmts.push(loop_stmt); + stmt_to_block(b.rules, Some(loop_stmt), &mut self.resolver) + } else { + //push `loop {}` onto the end of our fresh block and yield that + new_block.stmts.push(loop_stmt); - new_block - } - }) - } + new_block + } + }) } // in general the pretty printer processes unexpanded code, so From d8c99f3b1ab28ccadb3b56ba1fd506dce1385ee4 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 13:26:05 -0700 Subject: [PATCH 4/6] Preserve expressions that get a `DefId` Namely closures and `async` blocks. We have to make a few modifications to closures to make them compile. --- src/librustc_interface/util.rs | 83 +++++++++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 4d33383071e16..e446c8c0004c3 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -615,6 +615,12 @@ impl<'a, 'b> ReplaceBodyWithLoop<'a, 'b> { matches!(sig.header.constness, ast::Const::Yes(_)) || matches!(&sig.decl.output, ast::FnRetTy::Ty(ty) if ty.contains_impl_trait()) } + + /// Keep some `Expr`s that are ultimately assigned `DefId`s. This keeps the `HirId` parent + /// mappings correct even after this pass runs. + fn should_preserve_expr(e: &ast::Expr) -> bool { + matches!(&e.kind, ast::ExprKind::Closure(..) | ast::ExprKind::Async(..)) + } } impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { @@ -644,6 +650,75 @@ impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { self.run(true, |s| noop_visit_anon_const(c, s)) } + fn filter_map_expr(&mut self, mut e: P) -> Option> { + if self.ignore_item { + return noop_filter_map_expr(e, self); + } + + if let ast::ExprKind::Closure(.., decl, expr, _) = &mut e.kind { + // Replacing a closure body and removing its callsites will break inference. Give + // all closures the signature `|| -> !` to work around this. + decl.inputs = vec![]; + if let ast::FnRetTy::Default(_) = decl.output { + decl.output = ast::FnRetTy::Ty(P(ast::Ty { + id: self.resolver.next_node_id(), + span: rustc_span::DUMMY_SP, + kind: ast::TyKind::Never, + })); + } + + // Replace `|| expr` with `|| { expr }` so that `visit_block` gets called on the + // closure body. + if !matches!(expr.kind, ast::ExprKind::Block(..)) { + let new_stmt = ast::Stmt { + kind: ast::StmtKind::Expr(expr.clone()), + id: self.resolver.next_node_id(), + span: expr.span, + }; + + let new_block = ast::Block { + stmts: vec![new_stmt], + rules: BlockCheckMode::Default, + id: self.resolver.next_node_id(), + span: expr.span, + }; + + expr.kind = ast::ExprKind::Block(P(new_block), None); + } + } + + if Self::should_preserve_expr(&e) { + self.run(false, |s| noop_filter_map_expr(e, s)) + } else { + noop_filter_map_expr(e, self) + } + } + + fn flat_map_stmt(&mut self, s: ast::Stmt) -> SmallVec<[ast::Stmt; 1]> { + if self.ignore_item { + return noop_flat_map_stmt(s, self); + } + + let ast::Stmt { id, span, .. } = s; + match s.kind { + // Replace `let x = || {};` with `|| {};` + ast::StmtKind::Local(mut local) if local.init.is_some() => self + .filter_map_expr(local.init.take().unwrap()) + .into_iter() + .map(|e| ast::Stmt { kind: ast::StmtKind::Semi(e), id, span }) + .collect(), + + // Replace `|| {}` with `|| {};` + ast::StmtKind::Expr(expr) => self + .filter_map_expr(expr) + .into_iter() + .map(|e| ast::Stmt { kind: ast::StmtKind::Semi(e), id, span }) + .collect(), + + _ => noop_flat_map_stmt(s, self), + } + } + fn visit_block(&mut self, b: &mut P) { fn stmt_to_block( rules: ast::BlockCheckMode, @@ -699,7 +774,13 @@ impl<'a> MutVisitor for ReplaceBodyWithLoop<'a, '_> { for s in b.stmts { let old_blocks = self.nested_blocks.replace(vec![]); - stmts.extend(self.flat_map_stmt(s).into_iter().filter(|s| s.is_item())); + stmts.extend(self.flat_map_stmt(s).into_iter().filter(|s| { + s.is_item() + || matches!( + &s.kind, + ast::StmtKind::Semi(expr) if Self::should_preserve_expr(expr) + ) + })); // we put a Some in there earlier with that replace(), so this is valid let new_blocks = self.nested_blocks.take().unwrap(); From 1a300427536883739ea30cc3e2c72409dadae4e1 Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 15:03:24 -0700 Subject: [PATCH 5/6] Remove unnecessary `opt_local_def_id_to_hir_id` --- src/librustc_privacy/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index cb896810951ba..87b15853a087c 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -943,11 +943,8 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { let macro_module_def_id = ty::DefIdTree::parent(self.tcx, self.tcx.hir().local_def_id(md.hir_id).to_def_id()) .unwrap(); - // FIXME(#71104) Should really be using just `as_local_hir_id` but - // some `DefId` do not seem to have a corresponding HirId. - let hir_id = macro_module_def_id - .as_local() - .and_then(|def_id| self.tcx.hir().opt_local_def_id_to_hir_id(def_id)); + let hir_id = + macro_module_def_id.as_local().map(|def_id| self.tcx.hir().as_local_hir_id(def_id)); let mut module_id = match hir_id { Some(module_id) if self.tcx.hir().is_hir_id_module(module_id) => module_id, // `module_id` doesn't correspond to a `mod`, return early (#63164, #65252). From e319f203bd005282cf3eaefd577f35bd7cc1eecf Mon Sep 17 00:00:00 2001 From: Dylan MacKenzie Date: Sun, 7 Jun 2020 15:13:40 -0700 Subject: [PATCH 6/6] Add `rustdoc` tests from #72088 --- src/test/rustdoc/macro-in-async-block.rs | 9 +++++++++ src/test/rustdoc/macro-in-closure.rs | 7 +++++++ 2 files changed, 16 insertions(+) create mode 100644 src/test/rustdoc/macro-in-async-block.rs diff --git a/src/test/rustdoc/macro-in-async-block.rs b/src/test/rustdoc/macro-in-async-block.rs new file mode 100644 index 0000000000000..b4aaacf7b3d40 --- /dev/null +++ b/src/test/rustdoc/macro-in-async-block.rs @@ -0,0 +1,9 @@ +// Regression issue for rustdoc ICE encountered in PR #72088. +// edition:2018 +#![feature(decl_macro)] + +fn main() { + async { + macro m() {} + }; +} diff --git a/src/test/rustdoc/macro-in-closure.rs b/src/test/rustdoc/macro-in-closure.rs index 298ff601de89f..b4411d927e271 100644 --- a/src/test/rustdoc/macro-in-closure.rs +++ b/src/test/rustdoc/macro-in-closure.rs @@ -6,4 +6,11 @@ fn main() { || { macro m() {} }; + + let _ = || { + macro n() {} + }; + + let cond = true; + let _ = || if cond { macro n() {} } else { panic!() }; }