Remove missing_docs lint on private 2.0 macros

[inquisitivecrystal]

rust/compiler/rustc_lint/src/builtin.rs

Lines 573 to 584 in 798baeb

	for macro_def in krate.exported_macros {
	let attrs = cx.tcx.hir().attrs(macro_def.hir_id());
	let has_doc = attrs.iter().any(|a| has_doc(cx.sess(), a));
	if !has_doc {
	cx.struct_span_lint(
	MISSING_DOCS,
	cx.tcx.sess.source_map().guess_head_span(macro_def.span),
	|lint| lint.build("missing documentation for macro").emit(),
	);
	}
	}
	}

This code is the source of #57569. The problem is subtle, so let me point it out. This code makes the mistake of assuming that all of the macros in krate.exported_macros are exported.

...Yeah. For some historical reason, all macro macros are marked as exported, regardless of whether they actually are, which is dreadfully confusing. It would be more accurate to say that exported_macros currently contains only macros that have paths.

This PR renames exported_macros to importable_macros, since these macros can be imported with use while others cannot. It also fixes the code above to no longer lint on private macro macros, since the missing_docs lint should only appear on exported items.

Fixes #57569.

[rust-highfive]

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

[petrochenkov]

since these macros can be imported with use while others cannot.

Other macros can also be imported with use.
I suggest avoiding renaming in this PR.
Ideally, exported_macros and non_exported_macro_attrs should be removed entirely, and macro items should be kept in HIR as regular items, but it requires some work (#79396 (comment)).

[inquisitivecrystal]

since these macros can be imported with use while others cannot.

Other macros can also be imported with use.
I suggest avoiding renaming in this PR.
Ideally, exported_macros and non_exported_macro_attrs should be removed entirely, and macro items should be kept in HIR as regular items, but it requires some work (#79396 (comment)).

This field contains both 2.0 macros and exported macro_rules! macros. Pardon me if that was unclear, I think I explained it rather poorly. Procedural macros are also importable, but at the same time, they're exported too. I'm not entirely sure whether they're currently placed in exported_macros or not. Still, it would seem to me that this is strictly more correct and less confusing than the current system? Or am I missing something?

I looked at making macro items work like other items, but that's well above my pay grade.

[oli-obk]

I suggest avoiding renaming in this PR.

I don't have an opinion here, so let's keep the original name and figure out the details on renaming in an issue or zulip thread.

btw, thanks for doing this commit-by-commit, this PR was great to review.

[inquisitivecrystal]

I suggest avoiding renaming in this PR.

I don't have an opinion here, so let's keep the original name and figure out the details on renaming in an issue or zulip thread.

Alright! Which would be better, an issue or a Zulip thread? I can do either.

btw, thanks for doing this commit-by-commit, this PR was great to review.

Thank you!

[inquisitivecrystal]

Alright, the changes are done! I force-pushed to remove the commit that did the rename and adjust the other commits accordingly. My new changes are in b49936c. I made that addition to the comment, as well as changing the code to apply the exported check to macro_rules macros as well. There's no reason not to apply it to all macros, and it should also hopefully make my PR compatible with #78855, though I don't have an explicit test for that at present.

[oli-obk]

Alright! Which would be better, an issue or a Zulip thread? I can do either.

for discussion, probably a zulip thread

Let's move this PR on then:

@bors r+

[bors]

📌 Commit b49936c has been approved by oli-obk

[bors]

⌛ Testing commit b49936c with merge 8eae2eb...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 8eae2eb to master...