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.

Sign up for GitHub

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

Jump to bottom

Spruce up the docs of several queries related to the type/trait system and const eval #134787

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Spruce up the docs of several queries related to the type/trait system and const eval #134787

merged 1 commit into from
Dec 27, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented Dec 26, 2024 •
edited
Loading

  • Editorial
    • Proper rustdoc summary/synopsis line by making use of extra paragraphs: Leads to better rendered output on module pages, in search result lists and overall, too
    • Use rustdoc warning blocks for admonitions of the form "do not call / avoid calling this query directly"
    • Use intra-doc links of the form [`Self::$query`] to cross-link queries. Indeed, such links are generally a bit brittle due to the existence of TyCtxtFeed which only contains a subset of queries. Therefore the docs of feedable queries cannot cross-link to non-feedable ones. I'd say it's fine to use intra-doc links despite the potential/unlikely occasional future breakage (if a query with the aforementioned characteristics becomes feedable). Self:: is nicer than TyCtxt:: (which would be more stable) since it accounts for other contexts like TyCtxt{Feed,At,Ensure{,WithValue}}
  • Informative
    • Generally add, flesh out and correct some doc comments
    • Add Panic sections (to a few selected queries only). The lists of panics aren't necessarily exhaustive and focus on the more "obvious" or "important" panics.
    • Where applicable add a paragraph calling attention to the relevant #[rustc_*] TEST attribute

The one non-doc change (it's internal and not observable):
Be even more defensive in query constness's impl (spiritual follow-up to #134122) (see self review comment).

Fixes #133494.

r? any(compiler-errors, oli-obk)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2024

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 26, 2024
fmease
fmease commented Dec 26, 2024
View reviewed changes
compiler/rustc_const_eval/src/const_eval/fn_queries.rs
/// If it is an assoc method or function,
/// return if it has a `const` modifier. If it is an intrinsic, report whether said intrinsic
/// has a `rustc_const_{un,}stable` attribute. Otherwise, panic.
/// Checks whether a function-like thing is considered to be `const`.
fn constness(tcx: TyCtxt<'_>, def_id: LocalDefId) -> hir::Constness {
let node = tcx.hir_node_by_def_id(def_id);

match node {
Copy link
Member Author

@fmease fmease Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Since #134122 this fn/query is only meant to work on function-likes. However on master it also works on local1 impl2 associated constants and all foreign items (should be: foreign functions only).

This rectifies it.

Footnotes

  1. constness doesn't even get encoded in the metadata for associated constants (should_encode_constness). ↩

  2. Lol, trait associated constants were excluded! smh ↩

@fmease fmease force-pushed the spruce-up-queries branch 7 times, most recently from 505d7e4 to 8d08447 Compare December 26, 2024 15:00
@rust-log-analyzer

This comment has been minimized.

Sign in to view
compiler-errors
compiler-errors approved these changes Dec 26, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

r=me with or without nit

compiler/rustc_middle/src/query/mod.rs Outdated
///
/// [`DefId`]: rustc_hir::def_id::DefId
/// For type aliases (whether eager or lazy), this returns the underlying aliased type.
Copy link
Member

@compiler-errors compiler-errors Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Want to mention that it reveals the hidden type of an opaque, but type_of_opaque could be used to detect cycles? Outside of that they're functionally the same, since type_of just calls type_of_opaque.

@fmease
Copy link
Member Author

fmease commented Dec 27, 2024 •
edited
Loading

I've added a paragraph about hidden/opaque types to type_of as suggested.

For transparency, I've further tweaked the docs (compare) because writing is hard and I couldn't help myself.

@bors r=compiler-errors rollup (no observable functional changes)

@bors

This comment was marked as outdated.

Sign in to view
@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 27, 2024
@fmease

This comment was marked as outdated.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 27, 2024
@fmease

This comment was marked as outdated.

Sign in to view
@bors

This comment was marked as outdated.

Sign in to view
@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 27, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 27, 2024
@fmease

This comment was marked as outdated.

Sign in to view
@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 27, 2024
@fmease
Copy link
Member Author

fmease commented Dec 27, 2024

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Dec 27, 2024

📌 Commit 454c09e has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 27, 2024
@jieyouxu jieyouxu mentioned this pull request Dec 27, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 27, 2024
@bors
Auto merge of rust-lang#134822 - jieyouxu:rollup-5xuaq82, r=jieyouxu
6d3db55 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#134606 (ptr::copy: fix docs for the overlapping case)
 - rust-lang#134622 (Windows: Use WriteFile to write to a UTF-8 console)
 - rust-lang#134759 (compiletest: Remove the `-test` suffix from normalize directives)
 - rust-lang#134787 (Spruce up the docs of several queries related to the type/trait system and const eval)
 - rust-lang#134806 (rustdoc: use shorter paths as preferred canonical paths)
 - rust-lang#134815 (Sort triples by name in platform_support.md)
 - rust-lang#134816 (tools: fix build failure caused by PR rust-lang#134420)
 - rust-lang#134819 (Fix mistake in windows file open)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 72ef16f into rust-lang:master Dec 27, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 27, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 27, 2024
@rust-timer
Unrolled build for rust-lang#134787
3719fa9 
Rollup merge of rust-lang#134787 - fmease:spruce-up-queries, r=compiler-errors

Spruce up the docs of several queries related to the type/trait system and const eval

- Editorial
  - Proper rustdoc summary/synopsis line by making use of extra paragraphs: Leads to better rendered output on module pages, in search result lists and overall, too
  - Use rustdoc warning blocks for admonitions of the form "do not call / avoid calling this query directly"
  - Use intra-doc links of the form ``[`Self::$query`]`` to cross-link queries. Indeed, such links are generally a bit brittle due to the existence of `TyCtxtFeed` which only contains a subset of queries. Therefore the docs of `feedable` queries cannot cross-link to non-`feedable` ones. I'd say it's fine to use intra-doc links despite the potential/unlikely occasional future breakage (if a query with the aforementioned characteristics becomes `feedable`). `Self::` is nicer than `TyCtxt::` (which would be more stable) since it accounts for other contexts like `TyCtxt{Feed,At,Ensure{,WithValue}}`
- Informative
  - Generally add, flesh out and correct some doc comments
  - Add *Panic* sections (to a few selected queries only). The lists of panics aren't necessarily exhaustive and focus on the more "obvious" or "important" panics.
  - Where applicable add a paragraph calling attention to the relevant [`#[rustc_*]` TEST attribute](https://rustc-dev-guide.rust-lang.org/compiler-debugging.html#rustc_-test-attributes)

The one non-doc change (it's internal and not observable):
Be even more defensive in `query constness`'s impl (spiritual follow-up to rust-lang#134122) (see self review comment).

Fixes rust-lang#133494.

r\? **any**(compiler-errors, oli-obk)
@fmease fmease deleted the spruce-up-queries branch December 27, 2024 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.

TyCtxt::variances_of is misdocumented as applying to a generic parameter
7 participants
@fmease @rustbot @rust-log-analyzer @bors @RalfJung @compiler-errors @oli-obk