Skip to content

Implement RFC 3631: add rustdoc doc_cfg features #138907

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
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
1 change: 0 additions & 1 deletion compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,6 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
gate_doc!(
"experimental" {
cfg => doc_cfg
cfg_hide => doc_cfg_hide
masked => doc_masked
notable_trait => doc_notable_trait
}
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,17 @@ passes_doc_alias_start_end =
passes_doc_attr_not_crate_level =
`#![doc({$attr_name} = "...")]` isn't allowed as a crate-level attribute

passes_doc_cfg_hide_takes_list =
`#[doc(cfg_hide(...))]` takes a list of attributes
passes_doc_auto_cfg_expects_hide_or_show =
`only "hide" or "show" are allowed in "#[doc(auto_cfg(...))]"`

passes_doc_auto_cfg_hide_show_expects_list =
`#![doc(auto_cfg({$attr_name}(...)))]` expects a list of items

passes_doc_auto_cfg_hide_show_unexpected_item =
`#![doc(auto_cfg({$attr_name}(...)))]` only accepts identifiers or key/values items
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
`#![doc(auto_cfg({$attr_name}(...)))]` only accepts identifiers or key/values items
`#![doc(auto_cfg({$attr_name}(...)))]` only accepts identifiers or key/value items


passes_doc_auto_cfg_wrong_literal =
`expected boolean for #[doc(auto_cfg = ...)]`

passes_doc_expect_str =
doc {$attr_name} attribute expects a string: #[doc({$attr_name} = "a")]
Expand Down
65 changes: 50 additions & 15 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::cell::Cell;
use std::collections::hash_map::Entry;

use rustc_abi::{Align, ExternAbi, Size};
use rustc_ast::{AttrStyle, LitKind, MetaItemInner, MetaItemKind, MetaItemLit, ast};
use rustc_ast::{AttrStyle, LitKind, MetaItem, MetaItemInner, MetaItemKind, MetaItemLit, ast};
use rustc_attr_data_structures::{AttributeKind, ReprAttr, find_attr};
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, DiagCtxtHandle, IntoDiagArg, MultiSpan, StashKey};
Expand Down Expand Up @@ -1303,16 +1303,53 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

/// Check that the `#![doc(cfg_hide(...))]` attribute only contains a list of attributes.
///
fn check_doc_cfg_hide(&self, meta: &MetaItemInner, hir_id: HirId) {
if meta.meta_item_list().is_none() {
self.tcx.emit_node_span_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span(),
errors::DocCfgHideTakesList,
);
/// Check that the `#![doc(auto_cfg(..))]` attribute has expected input.
fn check_doc_auto_cfg(&self, meta: &MetaItem, hir_id: HirId) {
match &meta.kind {
MetaItemKind::Word => {}
MetaItemKind::NameValue(lit) => {
if !matches!(lit.kind, LitKind::Bool(_)) {
self.tcx.emit_node_span_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span,
errors::DocAutoCfgWrongLiteral,
);
}
}
MetaItemKind::List(list) => {
for item in list {
let Some(attr_name) = item.name() else { continue };
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 the else case emit an error since we've received an invalid entry for the auto_cfg list? E.g. doc(auto_cfg(42)) or doc(auto_cfg(foo::bar)).

Copy link
Member

Choose a reason for hiding this comment

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

Please add test cases for this missing error, along with the ones below.

if attr_name != sym::hide && attr_name != sym::show {
self.tcx.emit_node_span_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span,
errors::DocAutoCfgExpectsHideOrShow,
);
} else if let Some(list) = item.meta_item_list() {
for item in list {
if item.meta_item_list().is_some() {
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 we also error if there is no meta item list but the item is an invalid cfg (e.g. a boolean, or a multi-segment path)?

self.tcx.emit_node_span_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
item.span(),
errors::DocAutoCfgHideShowUnexpectedItem {
attr_name: attr_name.as_str(),
Copy link
Member

Choose a reason for hiding this comment

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

attr_name is either "hide" or "show" here, not the invalid cfg entry.

},
);
}
}
} else {
self.tcx.emit_node_span_lint(
INVALID_DOC_ATTRIBUTES,
hir_id,
meta.span,
errors::DocAutoCfgHideShowExpectsList { attr_name: attr_name.as_str() },
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

);
}
}
}
}
}

Expand Down Expand Up @@ -1375,10 +1412,8 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
self.check_attr_crate_level(attr, meta, hir_id);
}

Some(sym::cfg_hide) => {
if self.check_attr_crate_level(attr, meta, hir_id) {
self.check_doc_cfg_hide(meta, hir_id);
}
Some(sym::auto_cfg) => {
self.check_doc_auto_cfg(i_meta, hir_id);
}

Some(sym::inline | sym::no_inline) => {
Expand Down
20 changes: 18 additions & 2 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,24 @@ pub(crate) struct DocTestLiteral;
pub(crate) struct DocTestTakesList;

#[derive(LintDiagnostic)]
#[diag(passes_doc_cfg_hide_takes_list)]
pub(crate) struct DocCfgHideTakesList;
#[diag(passes_doc_auto_cfg_wrong_literal)]
pub(crate) struct DocAutoCfgWrongLiteral;

#[derive(LintDiagnostic)]
#[diag(passes_doc_auto_cfg_expects_hide_or_show)]
pub(crate) struct DocAutoCfgExpectsHideOrShow;

#[derive(LintDiagnostic)]
#[diag(passes_doc_auto_cfg_hide_show_expects_list)]
pub(crate) struct DocAutoCfgHideShowExpectsList<'a> {
pub attr_name: &'a str,
}

#[derive(LintDiagnostic)]
#[diag(passes_doc_auto_cfg_hide_show_unexpected_item)]
pub(crate) struct DocAutoCfgHideShowUnexpectedItem<'a> {
pub attr_name: &'a str,
}

#[derive(LintDiagnostic)]
#[diag(passes_doc_test_unknown_any)]
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ symbols! {
attributes,
audit_that,
augmented_assignments,
auto_cfg,
auto_traits,
autodiff,
automatically_derived,
Expand Down Expand Up @@ -1101,6 +1102,8 @@ symbols! {
hashset_iter_ty,
hexagon_target_feature,
hidden,
hidden_cfg,
Copy link
Member

Choose a reason for hiding this comment

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

Why'd you add this symbol? It seems to be unused unless I'm missing something.

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think we can also remove the hide_cfg symbol (not that it necessarily matters).

hide,
hint,
homogeneous_aggregate,
host,
Expand Down Expand Up @@ -1905,6 +1908,7 @@ symbols! {
shl_assign,
shorter_tail_lifetimes,
should_panic,
show,
shr,
shr_assign,
sig_dfl,
Expand Down
32 changes: 23 additions & 9 deletions library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,29 @@
issue_tracker_base_url = "https://github.com/rust-lang/rust/issues/",
test(no_crate_inject, attr(allow(unused_variables), deny(warnings)))
)]
#![doc(cfg_hide(
not(test),
not(any(test, bootstrap)),
no_global_oom_handling,
not(no_global_oom_handling),
not(no_rc),
not(no_sync),
target_has_atomic = "ptr"
))]
#![cfg_attr(
bootstrap,
doc(cfg_hide(
not(test),
not(any(test, bootstrap)),
no_global_oom_handling,
not(no_global_oom_handling),
not(no_rc),
not(no_sync),
target_has_atomic = "ptr"
))
)]
#![cfg_attr(
not(bootstrap),
doc(auto_cfg(hide(
bootstrap,
no_global_oom_handling,
no_global_oom_handling,
no_rc,
no_sync,
target_has_atomic = "ptr"
)))
)]
Comment on lines +67 to +89
Copy link
Member

Choose a reason for hiding this comment

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

  1. Why don't we hide test in the new version, even though the old (bootstrap) version does?
  2. Why is no_global_oom_handling listed twice in the new version?

#![doc(rust_logo)]
#![feature(rustdoc_internals)]
#![no_std]
Expand Down
69 changes: 48 additions & 21 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,54 @@
test(attr(allow(dead_code, deprecated, unused_variables, unused_mut)))
)]
#![doc(rust_logo)]
#![doc(cfg_hide(
no_fp_fmt_parse,
target_pointer_width = "16",
target_pointer_width = "32",
target_pointer_width = "64",
target_has_atomic = "8",
target_has_atomic = "16",
target_has_atomic = "32",
target_has_atomic = "64",
target_has_atomic = "ptr",
target_has_atomic_equal_alignment = "8",
target_has_atomic_equal_alignment = "16",
target_has_atomic_equal_alignment = "32",
target_has_atomic_equal_alignment = "64",
target_has_atomic_equal_alignment = "ptr",
target_has_atomic_load_store = "8",
target_has_atomic_load_store = "16",
target_has_atomic_load_store = "32",
target_has_atomic_load_store = "64",
target_has_atomic_load_store = "ptr",
))]
#![cfg_attr(
bootstrap,
doc(cfg_hide(
no_fp_fmt_parse,
target_pointer_width = "16",
target_pointer_width = "32",
target_pointer_width = "64",
target_has_atomic = "8",
target_has_atomic = "16",
target_has_atomic = "32",
target_has_atomic = "64",
target_has_atomic = "ptr",
target_has_atomic_equal_alignment = "8",
target_has_atomic_equal_alignment = "16",
target_has_atomic_equal_alignment = "32",
target_has_atomic_equal_alignment = "64",
target_has_atomic_equal_alignment = "ptr",
target_has_atomic_load_store = "8",
target_has_atomic_load_store = "16",
target_has_atomic_load_store = "32",
target_has_atomic_load_store = "64",
target_has_atomic_load_store = "ptr",
))
)]
#![cfg_attr(
not(bootstrap),
doc(auto_cfg(hide(
no_fp_fmt_parse,
target_pointer_width = "16",
target_pointer_width = "32",
target_pointer_width = "64",
target_has_atomic = "8",
target_has_atomic = "16",
target_has_atomic = "32",
target_has_atomic = "64",
target_has_atomic = "ptr",
target_has_atomic_equal_alignment = "8",
target_has_atomic_equal_alignment = "16",
target_has_atomic_equal_alignment = "32",
target_has_atomic_equal_alignment = "64",
target_has_atomic_equal_alignment = "ptr",
target_has_atomic_load_store = "8",
target_has_atomic_load_store = "16",
target_has_atomic_load_store = "32",
target_has_atomic_load_store = "64",
target_has_atomic_load_store = "ptr",
)))
)]
#![no_core]
#![rustc_coherence_is_core]
#![rustc_preserve_ub_checks]
Expand Down
16 changes: 10 additions & 6 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,16 @@
test(attr(allow(dead_code, deprecated, unused_variables, unused_mut)))
)]
#![doc(rust_logo)]
#![doc(cfg_hide(
not(test),
not(any(test, bootstrap)),
no_global_oom_handling,
not(no_global_oom_handling)
))]
#![cfg_attr(
bootstrap,
doc(cfg_hide(
not(test),
not(any(test, bootstrap)),
no_global_oom_handling,
not(no_global_oom_handling)
))
)]
#![cfg_attr(not(bootstrap), doc(auto_cfg(hide(bootstrap, no_global_oom_handling,))))]
// Don't link to std. We are std.
#![no_std]
// Tell the compiler to link to either panic_abort or panic_unwind
Expand Down
Loading
Loading