Skip to content

Commit 63a2f0f

Browse files
mejrsmejrs
committed
rework handling of doc attributes on macro calls
1 parent 7088e4b commit 63a2f0f

23 files changed

Lines changed: 404 additions & 145 deletions

‎Cargo.lock‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4035,7 +4035,6 @@ name = "rustc_expand"
40354035
version = "0.0.0"
40364036
dependencies = [
40374037
"rustc_ast",
4038-
"rustc_ast_passes",
40394038
"rustc_ast_pretty",
40404039
"rustc_attr_parsing",
40414040
"rustc_data_structures",

‎compiler/rustc_ast_lowering/src/lib.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ use rustc_ast::mut_visit::{self, MutVisitor};
4545
use rustc_ast::node_id::NodeMap;
4646
use rustc_ast::visit::{self, Visitor};
4747
use rustc_ast::{self as ast, *};
48-
use rustc_attr_parsing::{AttributeParser, OmitDoc, Recovery, ShouldEmit};
48+
use rustc_attr_parsing::{AttributeParser, Recovery, ShouldEmit};
4949
use rustc_data_structures::fx::FxIndexMap;
5050
use rustc_data_structures::sorted_map::SortedMap;
5151
use rustc_data_structures::stable_hash::{StableHash, StableHasher};
@@ -1228,7 +1228,6 @@ impl<'hir> LoweringContext<'_, 'hir> {
12281228
attrs,
12291229
target_span,
12301230
target,
1231-
OmitDoc::Lower,
12321231
|s| l.lower(s),
12331232
|lint_id, span, kind| {
12341233
self.delayed_lints.push(DelayedLint {

‎compiler/rustc_ast_passes/src/feature_gate.rs‎

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ macro_rules! gate_multi {
4747
}};
4848
}
4949

50-
pub fn check_attribute(attr: &ast::Attribute, sess: &Session, features: &Features) {
51-
PostExpansionVisitor { sess, features }.visit_attribute(attr)
52-
}
53-
5450
struct PostExpansionVisitor<'a> {
5551
sess: &'a Session,
5652

@@ -153,33 +149,9 @@ impl<'a> PostExpansionVisitor<'a> {
153149
}
154150

155151
impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
156-
fn visit_attribute(&mut self, attr: &ast::Attribute) {
157-
// Check unstable flavors of the `#[doc]` attribute.
158-
if attr.has_name(sym::doc) {
159-
for meta_item_inner in attr.meta_item_list().unwrap_or_default() {
160-
macro_rules! gate_doc { ($($s:literal { $($name:ident => $feature:ident)* })*) => {
161-
$($(if meta_item_inner.has_name(sym::$name) {
162-
let msg = concat!("`#[doc(", stringify!($name), ")]` is ", $s);
163-
gate!(self, $feature, attr.span, msg);
164-
})*)*
165-
}}
166-
167-
gate_doc!(
168-
"experimental" {
169-
cfg => doc_cfg
170-
auto_cfg => doc_cfg
171-
masked => doc_masked
172-
notable_trait => doc_notable_trait
173-
}
174-
"meant for internal use only" {
175-
attribute => rustdoc_internals
176-
keyword => rustdoc_internals
177-
fake_variadic => rustdoc_internals
178-
search_unbox => rustdoc_internals
179-
}
180-
);
181-
}
182-
}
152+
fn visit_attribute(&mut self, attr: &'a ast::Attribute) {
153+
// Checked in attribute parsers, do NOT add checks here
154+
visit::walk_attribute(self, attr)
183155
}
184156

185157
fn visit_item(&mut self, i: &'a ast::Item) {

‎compiler/rustc_attr_parsing/src/attributes/doc.rs‎

Lines changed: 124 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
1-
use rustc_ast::ast::{AttrStyle, LitKind, MetaItemLit};
1+
use rustc_ast::ExprKind;
2+
use rustc_ast::ast::{self, AttrArgs, AttrKind, AttrStyle, LitKind, MetaItemLit};
23
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, IndexEntry};
3-
use rustc_errors::{Applicability, msg};
4+
use rustc_errors::{Applicability, DiagArgValue, Diagnostic, MultiSpan};
45
use rustc_feature::AttributeStability;
56
use rustc_hir::Target;
67
use rustc_hir::attrs::{
78
AttributeKind, CfgEntry, CfgHideShow, DocAttribute, DocCfgHideShow, DocCfgHideShowValue,
89
DocInline, HideOrShow,
910
};
10-
use rustc_session::diagnostics::feature_err;
11+
use rustc_lint_defs::LintId;
1112
use rustc_span::{Span, Symbol, edition, sym};
1213

1314
use super::prelude::{ALL_TARGETS, AllowedTargets};
1415
use super::{AcceptMapping, AttributeParser, template};
16+
use crate::EmitAttribute;
1517
use crate::context::{AcceptContext, FinalizeContext};
1618
use crate::diagnostics::{
1719
AttrCrateLevelOnly, DocAliasBadChar, DocAliasDuplicated, DocAliasEmpty, DocAliasMalformed,
@@ -21,7 +23,8 @@ use crate::diagnostics::{
2123
DocAutoCfgHideShowValuesMix, DocAutoCfgWrongLiteral, DocKeywordNotKeyword, DocTestLiteral,
2224
DocTestTakesList, DocTestUnknown, DocUnknownAny, DocUnknownInclude, DocUnknownPasses,
2325
DocUnknownPlugins, DocUnknownSpotlight, ExpectedNameValue, ExpectedNoArgs,
24-
IllFormedAttributeInput, MalformedDoc, UnusedDuplicate,
26+
IllFormedAttributeInput, InvalidExprInDocAttrOnMacro, InvalidTarget, MalformedDoc,
27+
UnusedDuplicate,
2528
};
2629
use crate::parser::{
2730
ArgParser, MetaItemListParser, MetaItemOrLitParser, MetaItemParser, OwnedPathParser,
@@ -549,19 +552,15 @@ impl DocParser {
549552
}
550553
macro_rules! no_args_and_crate_level {
551554
($ident: ident) => {{
552-
no_args_and_crate_level!($ident, |span| {});
553-
}};
554-
($ident: ident, |$span:ident| $extra_validation:block) => {{
555555
if let Err(span) = args.as_no_args() {
556556
expected_no_args(cx, span);
557557
return;
558558
}
559-
let $span = path.span();
560-
if !check_attr_crate_level(cx, $span) {
559+
let span = path.span();
560+
if !check_attr_crate_level(cx, span) {
561561
return;
562562
}
563-
$extra_validation
564-
self.attribute.$ident = Some($span);
563+
self.attribute.$ident = Some(span);
565564
}};
566565
}
567566
macro_rules! string_arg_and_crate_level {
@@ -592,6 +591,12 @@ impl DocParser {
592591
self.attribute.$ident = Some((s, path.span()));
593592
}};
594593
}
594+
macro_rules! gated {
595+
($feature:ident $(,$notes:expr)*) => {
596+
let stability = $crate::unstable!($feature $(, $notes)*);
597+
cx.shared.cx.check_attribute_stability(&cx.attr_path, cx.attr_span, stability);
598+
};
599+
}
595600

596601
match path.word_sym() {
597602
Some(sym::alias) => self.parse_alias(cx, path, args),
@@ -606,37 +611,60 @@ impl DocParser {
606611
}
607612
Some(sym::inline) => self.parse_inline(cx, path, args, DocInline::Inline),
608613
Some(sym::no_inline) => self.parse_inline(cx, path, args, DocInline::NoInline),
609-
Some(sym::masked) => no_args!(masked),
610-
Some(sym::cfg) => self.parse_cfg(cx, args),
611-
Some(sym::notable_trait) => no_args!(notable_trait),
612-
Some(sym::keyword) => parse_keyword_and_attribute(
613-
cx,
614-
path,
615-
args,
616-
&mut self.attribute.keyword,
617-
sym::keyword,
618-
),
619-
Some(sym::attribute) => parse_keyword_and_attribute(
620-
cx,
621-
path,
622-
args,
623-
&mut self.attribute.attribute,
624-
sym::attribute,
625-
),
626-
Some(sym::fake_variadic) => no_args_and_not_crate_level!(fake_variadic),
627-
Some(sym::search_unbox) => no_args_and_not_crate_level!(search_unbox),
628-
Some(sym::rust_logo) => no_args_and_crate_level!(rust_logo, |span| {
629-
if !cx.features().rustdoc_internals() {
630-
feature_err(
631-
cx.sess(),
632-
sym::rustdoc_internals,
633-
span,
634-
msg!("the `#[doc(rust_logo)]` attribute is used for Rust branding"),
635-
)
636-
.emit();
614+
Some(sym::masked) => {
615+
gated!(doc_masked);
616+
no_args!(masked)
617+
}
618+
Some(sym::cfg) => {
619+
gated!(doc_cfg);
620+
self.parse_cfg(cx, args)
621+
}
622+
Some(sym::notable_trait) => {
623+
gated!(doc_notable_trait);
624+
no_args!(notable_trait)
625+
}
626+
Some(sym::keyword) => {
627+
gated!(rustdoc_internals);
628+
parse_keyword_and_attribute(
629+
cx,
630+
path,
631+
args,
632+
&mut self.attribute.keyword,
633+
sym::keyword,
634+
)
635+
}
636+
Some(sym::attribute) => {
637+
gated!(rustdoc_internals);
638+
parse_keyword_and_attribute(
639+
cx,
640+
path,
641+
args,
642+
&mut self.attribute.attribute,
643+
sym::attribute,
644+
)
645+
}
646+
Some(sym::fake_variadic) => {
647+
gated!(rustdoc_internals);
648+
no_args_and_not_crate_level!(fake_variadic)
649+
}
650+
Some(sym::search_unbox) => {
651+
gated!(rustdoc_internals);
652+
no_args_and_not_crate_level!(search_unbox)
653+
}
654+
Some(sym::rust_logo) => {
655+
// FIXME: Only feature gated at the crate level (!!)
656+
if cx.target == Target::Crate {
657+
gated!(
658+
rustdoc_internals,
659+
"the `#[doc(rust_logo)]` attribute is used for Rust branding"
660+
);
637661
}
638-
}),
639-
Some(sym::auto_cfg) => self.parse_auto_cfg(cx, path, args),
662+
no_args_and_crate_level!(rust_logo)
663+
}
664+
Some(sym::auto_cfg) => {
665+
gated!(doc_cfg);
666+
self.parse_auto_cfg(cx, path, args)
667+
}
640668
Some(sym::test) => {
641669
let Some(list) = args.as_list() else {
642670
cx.emit_lint(
@@ -797,6 +825,23 @@ impl AttributeParser for DocParser {
797825
),
798826
AttributeStability::Stable, // Some parts of the attribute are unstable, manually checked in parser
799827
|this, cx, args| {
828+
// FIXME: use actual target checking, see also `ALLOWED_TARGETS` comment
829+
if cx.target == Target::MacroCall {
830+
let attr_span = cx.attr_span;
831+
let diag = InvalidTarget {
832+
span: cx.inner_span,
833+
attr_span,
834+
name: cx.attr_path.clone(),
835+
target: cx.target.plural_name(),
836+
only: "",
837+
applied: DiagArgValue::Str("most other positions".into()),
838+
attribute_args: "(...)".to_string(),
839+
help: None,
840+
previously_accepted: false,
841+
on_macro_call: true,
842+
};
843+
cx.emit_lint(rustc_session::lint::builtin::UNUSED_ATTRIBUTES, diag, attr_span);
844+
}
800845
this.accept_single_doc_attr(cx, args);
801846
},
802847
)];
@@ -842,3 +887,41 @@ impl AttributeParser for DocParser {
842887
}
843888
}
844889
}
890+
891+
/// Is this a `#[doc = mac!()]`?
892+
///
893+
/// Or perhaps something as spicy as this?
894+
/// ```rust
895+
/// #[doc = {
896+
/// let a = 1;
897+
/// let b = 1;
898+
/// let sum = a + b;
899+
/// assert_eq!(sum, 2);
900+
/// }]
901+
/// println!();
902+
/// ```
903+
pub(crate) fn lint_non_lit_doc_attr(
904+
mut emit_lint: impl FnMut(LintId, MultiSpan, EmitAttribute),
905+
attr: &ast::Attribute,
906+
) -> bool {
907+
if !attr.has_name(sym::doc) {
908+
return false;
909+
}
910+
let AttrKind::Normal(n) = &attr.kind else { return false };
911+
let AttrArgs::Eq { expr, .. } = &n.item.args else { return false };
912+
if matches!(expr.kind, ExprKind::Lit(_)) {
913+
return false;
914+
};
915+
916+
let attr_span = attr.span;
917+
let expr_span = expr.span;
918+
919+
emit_lint(
920+
LintId::of(rustc_session::lint::builtin::UNUSED_ATTRIBUTES),
921+
attr_span.into(),
922+
EmitAttribute(Box::new(move |dcx, level, _| {
923+
InvalidExprInDocAttrOnMacro { attr_span, expr_span }.into_diag(dcx, level)
924+
})),
925+
);
926+
true
927+
}

‎compiler/rustc_attr_parsing/src/context.rs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -865,12 +865,6 @@ impl<'p, 'sess: 'p> DerefMut for SharedContext<'p, 'sess> {
865865
}
866866
}
867867

868-
#[derive(PartialEq, Clone, Copy, Debug)]
869-
pub enum OmitDoc {
870-
Lower,
871-
Skip,
872-
}
873-
874868
#[derive(Copy, Clone, Debug)]
875869
pub enum ShouldEmit {
876870
/// The operations will emit errors, and lints, and errors are fatal.

‎compiler/rustc_attr_parsing/src/diagnostics.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1335,6 +1335,26 @@ pub(crate) enum InvalidTargetHelp {
13351335
UseRustcAlignStatic,
13361336
}
13371337

1338+
#[derive(Diagnostic)]
1339+
#[diag(
1340+
"placing the `doc` attribute on a macro invocation does nothing even if the macro expands to what would be a valid target for the attribute"
1341+
)]
1342+
pub(crate) struct InvalidExprInDocAttrOnMacro {
1343+
#[primary_span]
1344+
#[suggestion(
1345+
"remove the attribute",
1346+
code = "",
1347+
applicability = "machine-applicable",
1348+
style = "tool-only"
1349+
)]
1350+
pub attr_span: Span,
1351+
1352+
#[warning(
1353+
"arbitrary expressions in doc attributes were previously accepted by the compiler but are being phased out; it will become a hard error in a future release!"
1354+
)]
1355+
pub expr_span: Span,
1356+
}
1357+
13381358
#[derive(Diagnostic)]
13391359
#[diag("invalid alignment value: {$error_part}", code = E0589)]
13401360
pub(crate) struct InvalidAlignmentValue {

0 commit comments

Comments
 (0)