Skip to content

Commit

Permalink
rustdoc-search: simplify rules for generics and type params
Browse files Browse the repository at this point in the history
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
  • Loading branch information
notriddle committed Aug 13, 2024
1 parent 36e26ab commit bbdd6e8
Show file tree
Hide file tree
Showing 38 changed files with 581 additions and 208 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
"meant for internal use only" {
keyword => rustdoc_internals
fake_variadic => rustdoc_internals
search_unbox => rustdoc_internals
}
);
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ passes_doc_masked_only_extern_crate =
passes_doc_rust_logo =
the `#[doc(rust_logo)]` attribute is used for Rust branding
passes_doc_search_unbox_invalid =
`#[doc(search_unbox)]` should be used on generic structs and enums
passes_doc_test_literal = `#![doc(test(...)]` does not take a literal
passes_doc_test_takes_list =
Expand Down
28 changes: 26 additions & 2 deletions compiler/rustc_passes/src/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ use rustc_ast::{
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, DiagCtxtHandle, IntoDiagArg, MultiSpan, StashKey};
use rustc_feature::{AttributeDuplicates, AttributeType, BuiltinAttribute, BUILTIN_ATTRIBUTE_MAP};
use rustc_hir as hir;
use rustc_hir::def_id::LocalModDefId;
use rustc_hir::intravisit::{self, Visitor};
use rustc_hir::{
self as hir, self, FnSig, ForeignItem, HirId, Item, ItemKind, MethodKind, Safety, Target,
TraitItem, CRATE_HIR_ID, CRATE_OWNER_ID,
self as hir, self, AssocItemKind, FnSig, ForeignItem, HirId, Item, ItemKind, MethodKind,
Safety, Target, TraitItem, CRATE_HIR_ID, CRATE_OWNER_ID,
};
use rustc_macros::LintDiagnostic;
use rustc_middle::hir::nested_filter;
Expand Down Expand Up @@ -962,6 +963,23 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

fn check_doc_search_unbox(&self, meta: &NestedMetaItem, hir_id: HirId) {
let hir::Node::Item(item) = self.tcx.hir_node(hir_id) else {
self.dcx().emit_err(errors::DocSearchUnboxInvalid { span: meta.span() });
return;
};
match item.kind {
ItemKind::Enum(_, generics) | ItemKind::Struct(_, generics)
if generics.params.len() != 0 => {}
ItemKind::Trait(_, _, generics, _, items)
if generics.params.len() != 0
|| items.iter().any(|item| matches!(item.kind, AssocItemKind::Type)) => {}
_ => {
self.dcx().emit_err(errors::DocSearchUnboxInvalid { span: meta.span() });
}
}
}

/// Checks `#[doc(inline)]`/`#[doc(no_inline)]` attributes.
///
/// A doc inlining attribute is invalid if it is applied to a non-`use` item, or
Expand Down Expand Up @@ -1180,6 +1198,12 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
}
}

sym::search_unbox => {
if self.check_attr_not_crate_level(meta, hir_id, "fake_variadic") {
self.check_doc_search_unbox(meta, hir_id);
}
}

sym::html_favicon_url
| sym::html_logo_url
| sym::html_playground_url
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,13 @@ pub struct DocKeywordOnlyImpl {
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_doc_search_unbox_invalid)]
pub struct DocSearchUnboxInvalid {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(passes_doc_inline_conflict)]
#[help]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1704,6 +1704,7 @@ symbols! {
saturating_add,
saturating_div,
saturating_sub,
search_unbox,
select_unpredictable,
self_in_typedefs,
self_struct_ctor,
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ mod thin;
#[lang = "owned_box"]
#[fundamental]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(bootstrap), doc(search_unbox))]
// The declaration of the `Box` struct must be kept in sync with the
// compiler or ICEs will happen.
pub struct Box<
Expand Down
1 change: 1 addition & 0 deletions library/core/src/future/future.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::task::{Context, Poll};
/// [`async`]: ../../std/keyword.async.html
/// [`Waker`]: crate::task::Waker
#[doc(notable_trait)]
#[cfg_attr(not(bootstrap), doc(search_unbox))]
#[must_use = "futures do nothing unless you `.await` or poll them"]
#[stable(feature = "futures_api", since = "1.36.0")]
#[lang = "future_trait"]
Expand Down
1 change: 1 addition & 0 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ use crate::{cmp, convert, hint, mem, slice};
#[lang = "Option"]
#[stable(feature = "rust1", since = "1.0.0")]
#[allow(clippy::derived_hash_with_manual_eq)] // PartialEq is manually implemented equivalently
#[cfg_attr(not(bootstrap), doc(search_unbox))]
pub enum Option<T> {
/// No value.
#[lang = "None"]
Expand Down
1 change: 1 addition & 0 deletions library/core/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ use crate::{convert, fmt, hint};
#[must_use = "this `Result` may be an `Err` variant, which should be handled"]
#[rustc_diagnostic_item = "Result"]
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(bootstrap), doc(search_unbox))]
pub enum Result<T, E> {
/// Contains the success value
#[lang = "Ok"]
Expand Down
12 changes: 7 additions & 5 deletions src/doc/rustdoc/src/read-documentation/search.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,29 +130,31 @@ pub trait MyTrait {
/// This function can be found using the following search queries:
///
/// MyTrait<First=u8, Second=u32> -> bool
/// MyTrait<u32, First=u8> -> bool
/// MyTrait<Second=u32> -> bool
/// MyTrait<u32, u8> -> bool
///
/// The following queries, however, will *not* match it:
///
/// MyTrait<First=u32> -> bool
/// MyTrait<u32, u32> -> bool
/// MyTrait<u32, First=u8> -> bool
/// MyTrait<u32, u8> -> bool
pub fn my_fn(x: impl MyTrait<First=u8, Second=u32>) -> bool { true }
```

Generics and function parameters are order-agnostic, but sensitive to nesting
Function parameters are order-agnostic, but sensitive to nesting
and number of matches. For example, a function with the signature
`fn read_all(&mut self: impl Read) -> Result<Vec<u8>, Error>`
will match these queries:

* `&mut Read -> Result<Vec<u8>, Error>`
* `Read -> Result<Vec<u8>, Error>`
* `Read -> Result<Error, Vec>`
* `Read -> Result<Vec<u8>>`
* `Read -> u8`

But it *does not* match `Result<Vec, u8>` or `Result<u8<Vec>>`.
But it *does not* match `Result<Vec, u8>` or `Result<u8<Vec>>`,
because those are nested incorrectly, and it does not match
`Result<Error, Vec<u8>>` or `Result<Error>`, because those are
in the wrong order.

To search for a function that accepts a function as a parameter,
like `Iterator::all`, wrap the nested signature in parenthesis,
Expand Down
Loading

0 comments on commit bbdd6e8

Please sign in to comment.