From 0bee80210d1e155b5502e51b693b6c478ed29efd Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Mon, 12 Oct 2020 19:50:12 +0200 Subject: Rustdoc: Fix macros 2.0 and built-in derives being shown at the wrong path. Fixes #74355 The issue with the built-in derives may be related to: https://github.com/rust-lang/rust/issues/55482#issuecomment-434035721 --- src/test/rustdoc/macro_pub_in_module.rs | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/test/rustdoc/macro_pub_in_module.rs (limited to 'src/test/rustdoc') diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs new file mode 100644 index 00000000000..2dfc6b41706 --- /dev/null +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -0,0 +1,9 @@ +//! See issue #74355 +#![crate_name = "krate"] +#![feature(decl_macro)] + +// @has krate/some_module/macro.my_macro.html +pub mod some_module { + // + pub macro my_macro() {} +} -- cgit 1.4.1-3-g733a5 From 7d03870882aa05fc4c600afa3585251f54d299c4 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Tue, 13 Oct 2020 20:25:19 +0200 Subject: Implement suggestions from code review. --- src/librustdoc/visit_ast.rs | 44 +++++++++++++++++---------------- src/test/rustdoc/macro_pub_in_module.rs | 16 +++++++++--- 2 files changed, 35 insertions(+), 25 deletions(-) (limited to 'src/test/rustdoc') diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 8508af080be..8b68a2bd65c 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -70,35 +70,36 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { ); top_level_module.is_crate = true; // Attach the crate's exported macros to the top-level module. - // In the case of macros 2.0 (`pub macro`), and for built-in `derive`s as well - // (_e.g._, `Copy`), these are wrongly bundled in there too, so we need to fix that by + // In the case of macros 2.0 (`pub macro`), and for built-in `derive`s or attributes as + // well (_e.g._, `Copy`), these are wrongly bundled in there too, so we need to fix that by // moving them back to their correct locations. krate.exported_macros.iter().for_each(|def| { - macro_rules! try_some {($($body:tt)*) => ({ - fn fn_once R> (f: F) -> F { f } - fn_once(|| Some({ $($body)* }))() - })} - // In the case of dummy items, some of the following operations may fail. We propagate - // that within a `?`-capturing block, so as to fallback to the basic behavior. - let containing_module_of_def = try_some! { + /// A return value of `None` signifies a fallback to the default behavior (locating + /// the macro at the root of the crate). + fn containing_mod_of_macro<'module, 'hir>( + def: &'_ rustc_hir::MacroDef<'_>, + tcx: TyCtxt<'_>, + top_level_module: &'module mut Module<'hir>, + ) -> Option<&'module mut Module<'hir>> { // The `def` of a macro in `exported_macros` should correspond to either: // - a `#[macro-export] macro_rules!` macro, - // - a built-in `derive` macro such as the ones in `::core`, + // - a built-in `derive` (or attribute) macro such as the ones in `::core`, // - a `pub macro`. // Only the last two need to be fixed, thus: if def.ast.macro_rules { return None; } - let macro_parent_module = self.cx.tcx.def_path({ + /* Because of #77828 we cannot do the simpler: + let macro_parent_module = tcx.def_path(tcx.parent_module(def.hir_id).to_def_id()); + // and instead have to do: */ + let macro_parent_module = tcx.def_path({ use rustc_middle::ty::DefIdTree; - self.cx - .tcx - /* Because of #77828 we cannot do the simpler: - .parent_module(def.hir_id).to_def_id() - // and instead have to do: */ - .parent(self.cx.tcx.hir().local_def_id(def.hir_id).to_def_id())? + tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id())? }); - let mut cur_mod = &mut top_level_module; + // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead, + // lookup the module by its name, by looking at each path segment one at a time. + // WARNING: this will probably break in the presence of re-exports or shadowing. + let mut cur_mod = top_level_module; for path_segment in macro_parent_module.data { let path_segment = path_segment.to_string(); cur_mod = cur_mod.mods.iter_mut().find(|module| { @@ -108,9 +109,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { ) })?; } - cur_mod - }; - if let Some(module) = containing_module_of_def { + Some(cur_mod) + } + + if let Some(module) = containing_mod_of_macro(def, self.cx.tcx, &mut top_level_module) { &mut module.macros } else { &mut top_level_module.macros diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs index 2dfc6b41706..035d2a8c441 100644 --- a/src/test/rustdoc/macro_pub_in_module.rs +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -1,9 +1,17 @@ //! See issue #74355 +#![feature(decl_macro, no_core, rustc_attrs)] #![crate_name = "krate"] -#![feature(decl_macro)] +#![no_core] -// @has krate/some_module/macro.my_macro.html -pub mod some_module { - // +pub mod inner { + // @has krate/inner/macro.my_macro.html pub macro my_macro() {} + + // @has krate/inner/macro.test.html + #[rustc_builtin_macro] + pub macro test($item:item) {} + + // @has krate/inner/macro.Clone.html + #[rustc_builtin_macro] + pub macro Clone($item:item) {} } -- cgit 1.4.1-3-g733a5 From d3a33eb1f9803da21460b675f2452b2784d9f63b Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Wed, 14 Oct 2020 21:12:03 +0200 Subject: Fix type/value namespace clashes + test for that --- src/librustdoc/visit_ast.rs | 80 +++++++++++------------ src/test/rustdoc/auxiliary/macro_pub_in_module.rs | 13 ++++ src/test/rustdoc/macro_pub_in_module.rs | 36 +++++++++- 3 files changed, 85 insertions(+), 44 deletions(-) create mode 100644 src/test/rustdoc/auxiliary/macro_pub_in_module.rs (limited to 'src/test/rustdoc') diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 8b68a2bd65c..ebad35f4e55 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -74,50 +74,46 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // well (_e.g._, `Copy`), these are wrongly bundled in there too, so we need to fix that by // moving them back to their correct locations. krate.exported_macros.iter().for_each(|def| { - /// A return value of `None` signifies a fallback to the default behavior (locating - /// the macro at the root of the crate). - fn containing_mod_of_macro<'module, 'hir>( - def: &'_ rustc_hir::MacroDef<'_>, - tcx: TyCtxt<'_>, - top_level_module: &'module mut Module<'hir>, - ) -> Option<&'module mut Module<'hir>> { - // The `def` of a macro in `exported_macros` should correspond to either: - // - a `#[macro-export] macro_rules!` macro, - // - a built-in `derive` (or attribute) macro such as the ones in `::core`, - // - a `pub macro`. - // Only the last two need to be fixed, thus: - if def.ast.macro_rules { - return None; - } - /* Because of #77828 we cannot do the simpler: - let macro_parent_module = tcx.def_path(tcx.parent_module(def.hir_id).to_def_id()); - // and instead have to do: */ - let macro_parent_module = tcx.def_path({ - use rustc_middle::ty::DefIdTree; - tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id())? - }); - // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead, - // lookup the module by its name, by looking at each path segment one at a time. - // WARNING: this will probably break in the presence of re-exports or shadowing. - let mut cur_mod = top_level_module; - for path_segment in macro_parent_module.data { - let path_segment = path_segment.to_string(); - cur_mod = cur_mod.mods.iter_mut().find(|module| { - matches!( - module.name, Some(symbol) - if symbol.with(|mod_name| mod_name == path_segment) - ) - })?; - } - Some(cur_mod) + let visit_macro = || self.visit_local_macro(def, None); + // The `def` of a macro in `exported_macros` should correspond to either: + // - a `#[macro-export] macro_rules!` macro, + // - a built-in `derive` (or attribute) macro such as the ones in `::core`, + // - a `pub macro`. + // Only the last two need to be fixed, thus: + if def.ast.macro_rules { + top_level_module.macros.push(visit_macro()); + return; } - - if let Some(module) = containing_mod_of_macro(def, self.cx.tcx, &mut top_level_module) { - &mut module.macros - } else { - &mut top_level_module.macros + let tcx = self.cx.tcx; + /* Because of #77828 we cannot do the simpler: + let macro_parent_module = tcx.def_path(tcx.parent_module(def.hir_id).to_def_id()); + // and instead have to do: */ + let macro_parent_module = tcx.def_path({ + use rustc_middle::ty::DefIdTree; + tcx.parent(tcx.hir().local_def_id(def.hir_id).to_def_id()).unwrap() + }); + // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead, + // lookup the module by its name, by looking at each path segment one at a time. + let mut cur_mod = &mut top_level_module; + for path_segment in macro_parent_module.data { + let path_segment_ty_ns = match path_segment.data { + rustc_hir::definitions::DefPathData::TypeNs(symbol) => symbol, + _ => { + // If the path segment is not from the type namespace + // (_e.g._, it can be from a value namespace in the case of `f::` in: + // `fn f() { pub macro m() {} }` + // then the item is not accessible, and should thus act as if it didn't + // exist (unless "associated macros" (inside an `impl`) were a thing…). + return; + } + }; + cur_mod = cur_mod + .mods + .iter_mut() + .find(|module| module.name == Some(path_segment_ty_ns)) + .unwrap(); } - .push(self.visit_local_macro(def, None)); + cur_mod.macros.push(visit_macro()); }); self.cx.renderinfo.get_mut().exact_paths = self.exact_paths; diff --git a/src/test/rustdoc/auxiliary/macro_pub_in_module.rs b/src/test/rustdoc/auxiliary/macro_pub_in_module.rs new file mode 100644 index 00000000000..fa987689ec6 --- /dev/null +++ b/src/test/rustdoc/auxiliary/macro_pub_in_module.rs @@ -0,0 +1,13 @@ +// edition:2018 + +#![feature(decl_macro)] +#![crate_name = "external_crate"] + +pub mod some_module { + /* == Make sure the logic is not affected by a re-export == */ + mod private { + pub macro external_macro() {} + } + // @has external_crate/some_module/macro.external_macro.html + pub use private::external_macro; +} diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs index 035d2a8c441..7d92246279f 100644 --- a/src/test/rustdoc/macro_pub_in_module.rs +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -1,11 +1,18 @@ +// aux-build:macro_pub_in_module.rs +// edition:2018 +// build-aux-docs +// @has external_crate/some_module/macro.external_macro.html + //! See issue #74355 #![feature(decl_macro, no_core, rustc_attrs)] #![crate_name = "krate"] #![no_core] +extern crate external_crate; + pub mod inner { - // @has krate/inner/macro.my_macro.html - pub macro my_macro() {} + // @has krate/inner/macro.raw_const.html + pub macro raw_const() {} // @has krate/inner/macro.test.html #[rustc_builtin_macro] @@ -14,4 +21,29 @@ pub mod inner { // @has krate/inner/macro.Clone.html #[rustc_builtin_macro] pub macro Clone($item:item) {} + + // Make sure the logic is not affected by a re-export. + mod private { + pub macro m() {} + } + // @has krate/inner/macro.renamed.html + pub use private::m as renamed; + + // @has krate/inner/macro.external_macro.html + pub use ::external_crate::some_module::external_macro; +} + +// Namespaces: Make sure the logic does not mix up a function name with a module name… +fn both_fn_and_mod() { + pub macro m() {} +} +pub mod both_fn_and_mod { + // @!has krate/both_fn_and_mod/macro.m.html +} + +const __: () = { + pub macro m() {} +}; +pub mod __ { + // @!has krate/__/macro.m.html } -- cgit 1.4.1-3-g733a5 From 28e9c6723a9111b649384ca909eded20c18c76a3 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sun, 27 Dec 2020 19:05:35 +0100 Subject: Update test assertions (showcases bug) --- src/test/rustdoc/auxiliary/macro_pub_in_module.rs | 2 +- src/test/rustdoc/macro_pub_in_module.rs | 36 +++++++++++++++++++---- 2 files changed, 31 insertions(+), 7 deletions(-) (limited to 'src/test/rustdoc') diff --git a/src/test/rustdoc/auxiliary/macro_pub_in_module.rs b/src/test/rustdoc/auxiliary/macro_pub_in_module.rs index fa987689ec6..137b1238600 100644 --- a/src/test/rustdoc/auxiliary/macro_pub_in_module.rs +++ b/src/test/rustdoc/auxiliary/macro_pub_in_module.rs @@ -8,6 +8,6 @@ pub mod some_module { mod private { pub macro external_macro() {} } - // @has external_crate/some_module/macro.external_macro.html + pub use private::external_macro; } diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs index 7d92246279f..d488f51cd49 100644 --- a/src/test/rustdoc/macro_pub_in_module.rs +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -1,49 +1,73 @@ // aux-build:macro_pub_in_module.rs // edition:2018 // build-aux-docs -// @has external_crate/some_module/macro.external_macro.html //! See issue #74355 #![feature(decl_macro, no_core, rustc_attrs)] #![crate_name = "krate"] #![no_core] +// @has external_crate/some_module/macro.external_macro.html extern crate external_crate; pub mod inner { // @has krate/inner/macro.raw_const.html + // @!has krate/macro.raw_const.html pub macro raw_const() {} // @has krate/inner/macro.test.html + // @!has krate/macro.test.html #[rustc_builtin_macro] pub macro test($item:item) {} // @has krate/inner/macro.Clone.html + // @!has krate/macro.Clone.html #[rustc_builtin_macro] pub macro Clone($item:item) {} - // Make sure the logic is not affected by a re-export. + // Make sure the logic is not affected by re-exports. + mod unrenamed { + // @!has krate/macro.unrenamed.html + #[rustc_macro_transparency = "semitransparent"] + pub macro unrenamed() {} + } + // @has krate/inner/macro.unrenamed.html + pub use unrenamed::unrenamed; + mod private { + // @!has krate/macro.m.html pub macro m() {} } // @has krate/inner/macro.renamed.html + // @!has krate/macro.renamed.html pub use private::m as renamed; + mod private2 { + // @!has krate/macro.m2.html + pub macro m2() {} + } + use private2 as renamed_mod; + // @has krate/inner/macro.m2.html + pub use renamed_mod::m2; + // @has krate/inner/macro.external_macro.html + // @!has krate/macro.external_macro.html pub use ::external_crate::some_module::external_macro; } // Namespaces: Make sure the logic does not mix up a function name with a module name… fn both_fn_and_mod() { - pub macro m() {} + // @!has krate/macro.in_both_fn_and_mod.html + pub macro in_both_fn_and_mod() {} } pub mod both_fn_and_mod { - // @!has krate/both_fn_and_mod/macro.m.html + // @!has krate/both_fn_and_mod/macro.in_both_fn_and_mod.html } const __: () = { - pub macro m() {} + // @!has krate/macro.in_both_const_and_mod.html + pub macro in_both_const_and_mod() {} }; pub mod __ { - // @!has krate/__/macro.m.html + // @!has krate/__/macro.in_both_const_and_mod.html } -- cgit 1.4.1-3-g733a5 From 8df60663cc5331f1c911766975619932b8e18bb0 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Sun, 27 Dec 2020 22:14:43 +0100 Subject: Enhance tests based on code review Co-authored-by: Joshua Nelson --- src/test/rustdoc/macro_pub_in_module.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/test/rustdoc') diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs index d488f51cd49..59e69625455 100644 --- a/src/test/rustdoc/macro_pub_in_module.rs +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -7,7 +7,8 @@ #![crate_name = "krate"] #![no_core] -// @has external_crate/some_module/macro.external_macro.html + // @has external_crate/some_module/macro.external_macro.html + // @!has external_crate/macro.external_macro.html extern crate external_crate; pub mod inner { -- cgit 1.4.1-3-g733a5 From 255f107cacb8927e72798313d69fa83d2e752a20 Mon Sep 17 00:00:00 2001 From: Daniel Henry-Mantilla Date: Tue, 5 Jan 2021 15:07:40 +0100 Subject: Fix ICE on `pub macro`s defined within a non-module type namespace. --- src/librustdoc/visit_ast.rs | 18 +++++++++++++----- src/test/rustdoc/macro_pub_in_module.rs | 8 ++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) (limited to 'src/test/rustdoc') diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 1e78a014048..2dc9c7bc758 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -93,6 +93,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { }); // HACK: rustdoc has no way to lookup `doctree::Module`s by their HirId. Instead, // lookup the module by its name, by looking at each path segment one at a time. + // Once #80415 is merged, this whole `for` loop research can be replaced by that. let mut cur_mod = &mut top_level_module; for path_segment in macro_parent_module.data { let path_segment_ty_ns = match path_segment.data { @@ -106,11 +107,18 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { continue 'exported_macros; } }; - cur_mod = cur_mod - .mods - .iter_mut() - .find(|module| module.name == Some(path_segment_ty_ns)) - .unwrap(); + // The obtained name in the type namespace may belong to something that is not + // a `mod`ule (_e.g._, it could be an `enum` with a `pub macro` defined within + // the block used for a discriminant. + if let Some(child_mod) = + cur_mod.mods.iter_mut().find(|module| module.name == Some(path_segment_ty_ns)) + { + cur_mod = child_mod; + } else { + // If the macro's parent def path is not exclusively made of module + // components, then it is not accessible (c.f. previous `continue`). + continue 'exported_macros; + } } cur_mod.macros.push((def, None)); } diff --git a/src/test/rustdoc/macro_pub_in_module.rs b/src/test/rustdoc/macro_pub_in_module.rs index 59e69625455..4fd85d68994 100644 --- a/src/test/rustdoc/macro_pub_in_module.rs +++ b/src/test/rustdoc/macro_pub_in_module.rs @@ -72,3 +72,11 @@ const __: () = { pub mod __ { // @!has krate/__/macro.in_both_const_and_mod.html } + +enum Enum { + Crazy = { + // @!has krate/macro.this_is_getting_weird.html; + pub macro this_is_getting_weird() {} + 42 + }, +} -- cgit 1.4.1-3-g733a5