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

[Rustdoc JSON] Primitive types in core are missing from the index #104064

Closed
LukeMathWalker opened this issue Nov 6, 2022 · 6 comments · Fixed by #106412
Closed

[Rustdoc JSON] Primitive types in core are missing from the index #104064

LukeMathWalker opened this issue Nov 6, 2022 · 6 comments · Fixed by #106412
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@LukeMathWalker
Copy link
Contributor

The issue

The id for i32 in the JSON documentation generated for core is 0:79316:78663.
It appears in the path:

"0:79316:78663": {
  "crate_id":0,
  "path":["core","i32"],
  "kind":"primitive"
}

but it is not present in the index, although it's referenced in the links section for many other items.
The same happens to the other primitives I checked - e.g. i64 (0:79317:78665) or char (0:79294:78252).

How to reproduce

Add the rust-docs-json component to your nightly toolchain, using

rustup component add rust-docs-json --toolchain nightly

You can then inspect core.json in .rustup/toolchains/nightly-<platform-triplet>/share/doc/rust/json/core.json

@LukeMathWalker LukeMathWalker added the C-bug Category: This is a bug. label Nov 6, 2022
@LukeMathWalker
Copy link
Contributor Author

@rustbot modify labels: +A-rustdoc-json +T-rustdoc

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 6, 2022
@aDotInTheVoid
Copy link
Member

Investigating this:

  1. This isn't caught by jsondoclint, although it probably should be. (We can easily add a check that all item's in paths with crate_id 0 (local) are also in in the index.
  2. jsondoclint does produce a number of other errors, which need to be investigated and fixed.
  3. Both the module and the type are present in both the index and paths section.
Python 3.10.7 (main, Nov 24 2022, 19:45:47) [GCC 12.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import json
>>> import pprint
>>> with open("./build/x86_64-unknown-linux-gnu/json-doc/core.json") as f:
...     j = json.load(f)
...
>>> pprint.pp([(k, v) for (k, v) in j["index"].items() if v["name"] == "i32"])
[('0:75939:717',
  {'id': '0:75939:717',
   'crate_id': 0,
   'name': 'i32',
   'span': {'filename': 'library/core/src/primitive_docs.rs',
            'begin': [1179, 0],
            'end': [1179, 15]},
   'visibility': 'public',
   'docs': 'The 32-bit signed integer type.',
   'links': {},
   'attrs': ['#[doc(primitive = "i32")]',
             '#[stable(feature = "rust1", since = "1.0.0")]'],
   'deprecation': None,
   'kind': 'primitive',
   'inner': {'name': 'i32',
             'impls': ['0:889',
                       # abridged
                       '0:23323']}}),
 ('0:67:717',
  {'id': '0:67:717',
   'crate_id': 0,
   'name': 'i32',
   'span': {'filename': 'library/core/src/num/shells/i32.rs',
            'begin': [1, 0],
            'end': [13, 19]},
   'visibility': 'public',
   'docs': 'Constants for the 32-bit signed integer type.\n'
           '\n'
           '*[See also the `i32` primitive type][i32].*\n'
           '\n'
           'New code should use the associated constants directly on the '
           'primitive type.',
   'links': {'i32': '0:75939:79042'},
   'attrs': ['#[path = "num/shells/i32.rs"]',
             '#![stable(feature = "rust1", since = "1.0.0")]',
             '#![deprecated(since = "TBD", note =\n'
             '"all constants in this module replaced by associated constants '
             'on `i32`")]'],
   'deprecation': {'since': 'TBD',
                   'note': 'all constants in this module replaced by '
                           'associated constants on `i32`'},
   'kind': 'module',
   'inner': {'is_crate': False,
             'items': ['0:23361:2593', '0:23362:2600'],
             'is_stripped': False}})]
>>> pprint.pp([(k,v) for k,v in j["paths"].items() if v["path"] == ["core", "i32"]])
[('0:67:717', {'crate_id': 0, 'path': ['core', 'i32'], 'kind': 'module'}),
 ('0:75939:79042',
  {'crate_id': 0, 'path': ['core', 'i32'], 'kind': 'primitive'})]
>>>

However the type has a different ID in each, but only in the last part. Therefor, it seems the issue may be somewhere in ID generation.

@aDotInTheVoid
Copy link
Member

MCVE:

#![feature(no_core)]
#![feature(rustc_attrs)]
#![feature(rustdoc_internals)]
#![no_core]
#![rustc_coherence_is_core]

//! Link to [i32][prim@i32]

#[doc(primitive = "i32")]
mod prim_i32 {}

produces (abridged)

{
  "crate_version": null,
  "external_crates": {},
  "format_version": 23,
  "includes_private": false,
  "index": {
    "0:0:1572": {
      "docs": "Link to [i32][prim@i32]",
      "id": "0:0:1572",
      "kind": "module",
      "links": {"prim@i32": "0:1:1571"},
      "name": "path_index_id"
    },
    "0:1:717": {
      "docs": null,
      "id": "0:1:717",
      "kind": "primitive",
      "links": {},
      "name": "i32"
    }
  },
  "paths": {
    "0:0:1572": {"kind": "module", "path": ["path_index_id"]},
    "0:1:1571": {"kind": "primitive", "path": ["path_index_id", "i32"]}
  },
  "root": "0:0:1572"
}

Which used both 0:1:1571 and 0:1:717 for i32

@GuillaumeGomez
Copy link
Member

Found the issue: the primitive type when defined in the current crate has a DefId pointing to the (private) module which has the #[doc(primitive)] attribute. So unfortunately, this fix doesn't work:

fn retrieve_primitive_defid_if_needed(tcx: TyCtxt<'_>, cache: &Cache, def_id: DefId) -> DefId {
    if matches!(tcx.def_kind(def_id), DefKind::Mod) &&
        let Some(prim) = tcx.get_attrs(def_id, sym::doc)
            .flat_map(|attr| attr.meta_item_list().unwrap_or_default())
            .filter(|attr| attr.has_name(sym::primitive))
            .find_map(|attr| attr.value_str()) {
        crate::clean::PrimitiveType::from_symbol(prim).and_then(|p| crate::clean::Type::Primitive(p).def_id(cache)).unwrap_or(def_id)
    } else {
        def_id
    }
}

// Then we call it into `JsonRenderer::convert_item`:
impl JsonRenderer<'_> {
    pub(super) fn convert_item(&self, item: clean::Item) -> Option<Item> {
        let deprecation = item.deprecation(self.tcx);
        let links = self
            .cache
            .intra_doc_links
            .get(&item.item_id)
            .into_iter()
            .flatten()
            .map(|clean::ItemLink { link, page_id, fragment, .. }| {
                let id = match fragment {
                    Some(UrlFragment::Item(frag_id)) => {
                        // We need this work-around because links to primitive.
                        retrieve_primitive_defid_if_needed(self.tcx, &self.cache, *frag_id)
                    },
                    // FIXME: Pass the `UserWritten` segment to JSON consumer.
                    Some(UrlFragment::UserWritten(_)) | None => {
                        // We need this work-around because links to primitive.
                        retrieve_primitive_defid_if_needed(self.tcx, &self.cache, *page_id)
                    }
                };
                (link.clone(), from_item_id(id.into(), self.tcx))
            })
            .collect();

because the DefId returned is the module's which would be pretty useless in our case. The only way around this issue is to unfortunately "lie" when generating the ID... Anyway, I'm sending an ugly fix and we can eventually discuss an alternative but I'm afraid we're stuck here because of how we generate IDs.

@jyn514
Copy link
Member

jyn514 commented Jan 4, 2023

I would expect the proper fix to be adding the docs for the module to the id for kind: primitive. @GuillaumeGomez it looks like in #106412 you generated an entirely new ID from scratch? Why was that necessary?

@GuillaumeGomez
Copy link
Member

I actually switched the primitive module ID (which isn't present in the output because it's private) with the primitive type directly (which is present in the output). However if it's defined in the same crate, it's not treated as a primitive type, not sure if it's intended or not. Since the ID is the Symbol value, it works just fine in this case.

@bors bors closed this as completed in d214402 Jan 5, 2023
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 8, 2023
…al-item, r=notriddle

jsondoclint: Check local items in `paths` are also in `index`.

Would have caught rust-lang#104064 (if core.json was linted in CI).

Closes rust-lang#106433.

r? rustdoc
JohnTitor pushed a commit to JohnTitor/rust that referenced this issue Jan 8, 2023
…al-item, r=notriddle

jsondoclint: Check local items in `paths` are also in `index`.

Would have caught rust-lang#104064 (if core.json was linted in CI).

Closes rust-lang#106433.

r? rustdoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants