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.

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

#[doc(inline)] sym_generated #80523

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Conversation

LeSeulArtichaut
Copy link
Contributor

Manually doc-inlines rustc_span::sym_generated into sym.
Previously the docs would not get inlined, causing the symbols to be undocumented as sym_generated is private.

r? @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 30, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 30, 2020

Can you show a screenshot of what the new docs look like? The fastest way I know is to rebase over #79540 and then x.py doc --stage 0 compiler/rustc, but if you're not feeling adventurous, x.py doc --stage 1 compiler/rustc will work fine. You'll have to set compiler-docs = true in config.toml first.

@jyn514 jyn514 added A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 30, 2020
@jyn514
Copy link
Member

jyn514 commented Dec 30, 2020

Also just for the record: I expect this is a bug in rustdoc related to #77862 somehow, but this fix seems fine in the meantime.

@crlf0710 crlf0710 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 15, 2021
@crlf0710
Copy link
Member

@LeSeulArtichaut Ping from triage! What's the current status of this?

@jyn514
Copy link
Member

jyn514 commented Jan 15, 2021

Hmm, it looks like #77862 did not fix this: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/sym/index.html
@LeSeulArtichaut could you open a rustdoc issue? This workaround seems fine in the meantime.

@LeSeulArtichaut
Copy link
Contributor Author

LeSeulArtichaut commented Jan 16, 2021

Here's a screenshot of ./x.py doc --stage 0 compiler/rustc with this PR:

(Sorry for taking so much time 😕)

@LeSeulArtichaut
Copy link
Contributor Author

Hmm, it looks like #77862 did not fix this: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/sym/index.html
@LeSeulArtichaut could you open a rustdoc issue? This workaround seems fine in the meantime.

Should I also leave a FIXME referencing the issue?

@jyn514
Copy link
Member

jyn514 commented Jan 16, 2021

Hmm, it looks like #77862 did not fix this: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/symbol/sym/index.html
@LeSeulArtichaut could you open a rustdoc issue? This workaround seems fine in the meantime.

Should I also leave a FIXME referencing the issue?

Sure, sounds good to me, thanks :)

@petrochenkov
Copy link
Contributor

FWIW, the module kw above has an exactly equivalent reexport (pub use super::kw_generated::*;), it should probably be treated in the same way.

@camelid
Copy link
Member

camelid commented Feb 12, 2021

Is this still waiting on the author?

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

Yes, @LeSeulArtichaut can you please also add #[doc(inline)] to pub use kw_generated::*? r=me after that.

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

If you have time to open an issue that would be helpful too, but I won't block on that.

@LeSeulArtichaut
Copy link
Contributor Author

kw does render correctly though

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

Well here's the issue:

$ cargo expand
pub mod symbol {
    //! An "interner" is a data structure that associates values with usize tags and
    //! allows bidirectional lookup; i.e., given a value, one can easily find the
    //! type, and vice versa.
    use rustc_arena::DroplessArena;
    use rustc_data_structures::fx::FxHashMap;
    use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
    use rustc_macros::HashStable_Generic;
    use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
    use std::cmp::{Ord, PartialEq, PartialOrd};
    use std::fmt;
    use std::hash::{Hash, Hasher};
    use std::str;
    use crate::{Edition, Span, DUMMY_SP, SESSION_GLOBALS};
    const SYMBOL_DIGITS_BASE: u32 = 1211u32;
    #[doc(hidden)]
    #[allow(non_upper_case_globals)]
    mod kw_generated {
        // ...

Can you change the proc-macro to remove doc(hidden) instead?

cc #81980.

@LeSeulArtichaut
Copy link
Contributor Author

For both sym and kw? Or just sym?

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2021

Oh hmm, now I'm confused again - doc(hidden) is on both module, but only one of them is hidden. I don't know if it's worth looking into this further, up to you.

@bors delegate=LeSeulArtichaut

@bors
Copy link
Contributor

bors commented Feb 12, 2021

✌️ @LeSeulArtichaut can now approve this pull request

@LeSeulArtichaut
Copy link
Contributor Author

With exams coming up, I won't have time to investigate this in the near future. I think this should be merged so the documentation can be improved immediately.

@bors r=jyn514 rollup

@bors
Copy link
Contributor

bors commented Feb 13, 2021

📌 Commit 240907b has been approved by jyn514

@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 Feb 13, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 14, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#80523 (#[doc(inline)] sym_generated)
 - rust-lang#80920 (Visit more targets when validating attributes)
 - rust-lang#81720 (Updated smallvec version due to RUSTSEC-2021-0003)
 - rust-lang#81891 ([rustdoc-json] Make `header` a vec of modifiers, and FunctionPointer consistent)
 - rust-lang#81912 (Implement the precise analysis pass for lint `disjoint_capture_drop_reorder`)
 - rust-lang#81914 (Fixing bad suggestion for `_` in `const` type when a function rust-lang#81885)
 - rust-lang#81919 (BTreeMap: fix internal comments)
 - rust-lang#81927 (Add a regression test for rust-lang#32498)
 - rust-lang#81965 (Fix MIR pretty printer for non-local DefIds)
 - rust-lang#82029 (Use debug log level for developer oriented logs)
 - rust-lang#82056 (fix ice (rust-lang#82032))

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 29ed864 into rust-lang:master Feb 15, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 15, 2021
@LeSeulArtichaut LeSeulArtichaut deleted the inline-sym branch February 15, 2021 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself 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
Development

Successfully merging this pull request may close these issues.

8 participants