rustdoc-json: Don't ignore impls for primitive types

[hkmatsumoto]

Fix the issue discussed at Zulip

r? @jyn514

[jyn514]

Looking back it's unclear to me what problem this is solving. get_impls is only meant to show impls from the current crate, right? Why do you need it to be shown for non-local crates? Can you add a test showing the change in behavior?

[hkmatsumoto]

get_impls in json backend returns list of id linked to the impls for local items, and inserts the impls to the index. Using the latter feature here we have the information of impls of structs and enums in the resulting json file.

[jyn514]

#87073 has been merged, so this is no longer blocked :)

[hkmatsumoto]

I found a weird behavior with doc(primitive).

By running cargo +stage1 rustdoc -- --output-format json -Z unstable-options on the following code (using stage1 here because this part of change is needed to generate json file without panic):

#![feature(doc_primitive)]
#[doc(primitive = "char")]
mod char {}

cat target/doc/foo.json | jq '.index[] | select(.id == "0:0")' emits:

{
  "id": "0:0",
  "crate_id": 0,
  "name": "foo",
  "span": {
    "filename": "src/lib.rs",
    "begin": [
      1,
      0
    ],
    "end": [
      3,
      11
    ]
  },
  "visibility": "public",
  "docs": null,
  "links": {},
  "attrs": [
    "#![feature(doc_primitive)]"
  ],
  "deprecation": null,
  "kind": "module",
  "inner": {
    "is_crate": true,
    "items": [
      "0:3" // I assume this refers to `char` module but it's private
    ]
  }
}

while cat target/doc/issue_87073.json | jq '.index[] | select(.id == "0:3")' | wc -l emits 0, i.e., an item with the id is not present in index.

[hkmatsumoto]

I'm not sure exactly what's going wrong, but it's likely because the module itself is private. I think we may need to add a new type of item to the index to allow representing this? I don't think it has a type for primitives currently.

Added ItemEnum::Primitive(String) variant and it works like a charm. I wonder if we need to upgrade Crate::format_version due to this change.

[jyn514]

Can you also test the behavior when the primitive is only defined in libstd, not the current crate?

[hkmatsumoto]

Added tests to check crate_id of methods of usize does not match to local crate's. We cannot check if they come from libstd because the std module item is not in the index, but hope it suffices.

[jyn514]

Added tests to check crate_id of methods of usize does not match to local crate's.

In that case, this mod usize is doing nothing and should be removed.

[hkmatsumoto]

doc(primitive) requires an item to be defined right after itself, so we need this meaningless mod usize like we do in library/std/src/primitive_docs.rs.

[jyn514]

No, my point is that doc(primitive) is doing nothing. If it's using the methods in libstd then whether the docs come from this crate or libstd doesn't matter.

[jyn514]

Actually, I'm not sure this is the right approach at all - the original problem was that documenting libcore wouldn't show the primitive impls, right? but we don't want to show them on all crates unconditionally, only crates where the primitive is documented.

[jyn514]

Added ItemEnum::Primitive(String) variant and it works like a charm. I wonder if we need to upgrade Crate::format_version due to this change.

Sure, that seems useful.

How were primitives being handled before? I'm surprised it didn't ICE trying to document the primitives in libstd.

[hkmatsumoto]

How were primitives being handled before? I'm surprised it didn't ICE trying to document the primitives in libstd.

Definitions of primitive types themselves were not indexed, unlike those of user-defined ADTs. Impls for primitive types were not indexed, too (They'll be indexed after this patch).

[jyn514]

Definitions of primitive types themselves were not indexed, unlike those of user-defined ADTs.

Ok. Shouldn't those definitions be indexed too? It seems strange to show only the impls and not the primitives themselves.

[hkmatsumoto]

IIRC lang_items are not explicitly defined because the compiler does some injections instead.

[jyn514]

@hkmatsumoto right, but primitives aren't documented through lang_items. They're documented through doc(primitive) modules, like the one you wrote in the test. I'm asking what happens to those doc(primitive) modules. The correct behavior is for them to show up in the JSON index.

[hkmatsumoto]

With this code:

#![feature(doc_primitive)]

#[doc(primitive = "usize")]
mod foo {}

cat target/doc/issue_88234.json | jq '.index[] | select(.name == "primitive")' emits:

{
  "id": "0:0",
  "crate_id": 0,
  "name": "primitive",
  "span": {
    "filename": "src/lib.rs",
    "begin": [
      1,
      0
    ],
    "end": [
      4,
      11
    ]
  },
  "visibility": "public",
  "docs": null,
  "links": {},
  "attrs": [
    "#![feature(doc_primitive)]"
  ],
  "deprecation": null,
  "kind": "module",
  "inner": {
    "is_crate": true,
    "items": [
      "0:3"
    ]
  }
}

cat target/doc/issue_88234.json | jq '.index[] | select(.id == "0:3")' emits:

{
  "id": "0:3",
  "crate_id": 0,
  "name": "usize",
  "span": {
    "filename": "src/lib.rs",
    "begin": [
      4,
      0
    ],
    "end": [
      4,
      10
    ]
  },
  "visibility": "crate",
  "docs": null,
  "links": {},
  "attrs": [
    "#[doc(primitive = \"usize\")]"
  ],
  "deprecation": null,
  "kind": "primitive_type",
  "inner": "usize"
}

The modules are showing up.

[jyn514]

@bors r+

[bors]

📌 Commit e18a8ef has been approved by jyn514