Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix macro_rules! duplication when reexported in the same module #89867

Merged
merged 6 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,21 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
// the rexport defines the path that a user will actually see. Accordingly,
// we add the rexport as an item here, and then skip over the original
// definition in `visit_item()` below.
//
// We also skip `#[macro_export] macro_rules!` that have already been inserted,
// it can happen if within the same module a `#[macro_export] macro_rules!`
// is declared but also a reexport of itself producing two exports of the same
// macro in the same module.
let mut inserted = FxHashSet::default();
for export in self.cx.tcx.module_exports(CRATE_DEF_ID).unwrap_or(&[]) {
if let Res::Def(DefKind::Macro(_), def_id) = export.res {
if let Some(local_def_id) = def_id.as_local() {
if self.cx.tcx.has_attr(def_id, sym::macro_export) {
let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
let item = self.cx.tcx.hir().expect_item(hir_id);
top_level_module.items.push((item, None));
if inserted.insert(def_id) {
let hir_id = self.cx.tcx.hir().local_def_id_to_hir_id(local_def_id);
let item = self.cx.tcx.hir().expect_item(hir_id);
top_level_module.items.push((item, None));
}
}
}
}
Expand Down
17 changes: 17 additions & 0 deletions src/test/rustdoc-json/reexport/macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// edition:2018
Urgau marked this conversation as resolved.
Show resolved Hide resolved

#![no_core]
#![feature(no_core)]

// @count macro.json "$.index[*][?(@.name=='macro')].inner.items[*]" 2

// @set repro_id = macro.json "$.index[*][?(@.name=='repro')].id"
// @has - "$.index[*][?(@.name=='macro')].inner.items[*]" $repro_id
#[macro_export]
macro_rules! repro {
() => {};
}

// @set repro2_id = macro.json "$.index[*][?(@.inner.name=='repro2')].id"
// @has - "$.index[*][?(@.name=='macro')].inner.items[*]" $repro2_id
pub use crate::repro as repro2;
14 changes: 14 additions & 0 deletions src/test/rustdoc/issue-89852.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// edition:2018

#![no_core]
#![feature(no_core)]

// @matches 'issue_89852/sidebar-items.js' '"repro"'
// @!matches 'issue_89852/sidebar-items.js' '"repro".*"repro"'
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't repro2 be showing up in the sidebar as well?

Copy link
Member Author

@Urgau Urgau Oct 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but this would be a pre-existing issue that my PR does not try to solve.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use count instead to ensure there is only 1 repro? I'm really not a big fan of negative checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, but this would be a pre-existing issue that my PR does not try to solve.

Hmm, I wonder if they are related issues though: #89852 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you use count instead to ensure there is only 1 repro? I'm really not a big fan of negative checks.

Unfortunately no because it's a javascript and XPath doesn't work on them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it'll be enough for now then. It would have been much simpler to check using rustdoc-gui though... :-/


#[macro_export]
macro_rules! repro {
() => {};
}

pub use crate::repro as repro2;