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 might not report all implemented traits on a public type #100252

Closed
obi1kenobi opened this issue Aug 8, 2022 · 7 comments · Fixed by #100325
Closed

Rustdoc JSON might not report all implemented traits on a public type #100252

obi1kenobi opened this issue Aug 8, 2022 · 7 comments · Fixed by #100325
Labels
A-rustdoc-json Area: Rustdoc JSON backend C-bug Category: This is a bug. requires-nightly This issue requires a nightly compiler in some way. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@obi1kenobi
Copy link
Member

Given a public type, I expected all traits implemented by that type to be part of rustdoc JSON output.

However, rustdoc JSON output sometimes does not include information about all the traits implemented by a type unless --document-private-items is specified.

For example, in this clap semver regression:

  • The following invocation on v3.1.18 generates JSON stating that UnwindSafe is implemented for ArgMatches: cargo rustdoc --lib --all-features -- --document-private-items -Zunstable-options --output-format json
  • However, the following invocation has no mention of UnwindSafe for ArgMatches at all: cargo rustdoc --lib --all-features -- -Zunstable-options --output-format json

Related to:
obi1kenobi/cargo-semver-checks#32

Meta

rustc --version --verbose:

rustc 1.64.0-nightly (93ffde6f0 2022-07-23)
binary: rustc
commit-hash: 93ffde6f04d3d24327a4e17a2a2bf4f63c246235
commit-date: 2022-07-23
host: x86_64-unknown-linux-gnu
release: 1.64.0-nightly
LLVM version: 14.0.6
@obi1kenobi obi1kenobi added the C-bug Category: This is a bug. label Aug 8, 2022
@camelid camelid added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-json Area: Rustdoc JSON backend requires-nightly This issue requires a nightly compiler in some way. labels Aug 8, 2022
@camelid
Copy link
Member

camelid commented Aug 8, 2022

cc @CraftSpider @aDotInTheVoid

@aDotInTheVoid
Copy link
Member

@rustbot modify labels: +E-needs-mcve

@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Aug 8, 2022
@aDotInTheVoid
Copy link
Member

Interestingly, rustdoc-json is providing no impls at all:

"0:4070:1595": {
      "attrs": [],
      "crate_id": 0,
      "deprecation": null,
      "docs": "Container for parse results.\n\nUsed to get information about the arguments that were supplied to the program at runtime by\nthe user. New instances of this struct are obtained by using the [`Command::get_matches`] family of\nmethods.\n\n# Examples\n\n```no_run\n# use clap::{Command, Arg};\nlet matches = Command::new(\"MyApp\")\n    .arg(Arg::new(\"out\")\n        .long(\"output\")\n        .required(true)\n        .takes_value(true))\n    .arg(Arg::new(\"debug\")\n        .short('d')\n        .multiple_occurrences(true))\n    .arg(Arg::new(\"cfg\")\n        .short('c')\n        .takes_value(true))\n    .get_matches(); // builds the instance of ArgMatches\n\n// to get information about the \"cfg\" argument we created, such as the value supplied we use\n// various ArgMatches methods, such as ArgMatches::value_of\nif let Some(c) = matches.value_of(\"cfg\") {\n    println!(\"Value for -c: {}\", c);\n}\n\n// The ArgMatches::value_of method returns an Option because the user may not have supplied\n// that argument at runtime. But if we specified that the argument was \"required\" as we did\n// with the \"out\" argument, we can safely unwrap because `clap` verifies that was actually\n// used at runtime.\nprintln!(\"Value for --output: {}\", matches.value_of(\"out\").unwrap());\n\n// You can check the presence of an argument\nif matches.is_present(\"out\") {\n    // Another way to check if an argument was present, or if it occurred multiple times is to\n    // use occurrences_of() which returns 0 if an argument isn't found at runtime, or the\n    // number of times that it occurred, if it was. To allow an argument to appear more than\n    // once, you must use the .multiple_occurrences(true) method, otherwise it will only return 1 or 0.\n    if matches.occurrences_of(\"debug\") > 2 {\n        println!(\"Debug mode is REALLY on, don't be crazy\");\n    } else {\n        println!(\"Debug mode kind of on\");\n    }\n}\n```\n[`Command::get_matches`]: crate::Command::get_matches()",
      "id": "0:4070:1595",
      "inner": {
        "fields": [],
        "fields_stripped": true,
        "generics": {"params": [], "where_predicates": []},
        "impls": [],
        "struct_type": "plain"
      },
      "kind": "struct",
      "links": {"crate::Command::get_matches()": "0:3723:1609"},
      "name": "ArgMatches",
      "span": {
        "begin": [70, 0],
        "end": [79, 1],
        "filename": "src/parse/matches/arg_matches.rs"
      },
      "visibility": "public"
    },

@Enselic
Copy link
Member

Enselic commented Aug 8, 2022

Missing impls is probably a recent regression, see #100204

@aDotInTheVoid
Copy link
Member

MCVE

mod arg_matches {
    #[derive(Debug, Clone, Default, PartialEq, Eq)]
    pub struct ArgMatches();
}
pub use arg_matches::ArgMatches;

produces (full)

{
  "index": {
    "0:0:1564": {
      "id": "0:0:1564",
      "inner": {
        "is_crate": true,
        "is_stripped": false,
        "items": ["0:4"]
      },
      "kind": "module",
      "links": {},
      "name": "clap",
      "visibility": "public"
    },
    "0:4": {
      "id": "0:4",
      "inner": {"glob": false, "id": "0:7:1563", "name": "ArgMatches", "source": "arg_matches::ArgMatches"},
      "kind": "import",
      "name": null,
      "visibility": "public"
    },
    "0:7:1563": {
      "id": "0:7:1563",
      "inner": {"fields": [], "fields_stripped": false, "impls": [], "struct_type": "tuple"},
      "kind": "struct",
      "name": "ArgMatches",
      "visibility": "public"
    }
  },
  "root": "0:0:1564"
}

and with --document-private-items (full)

{
  "index": {
    "0:0:1564": {
      "id": "0:0:1564",
      "inner": {
        "is_crate": true,
        "is_stripped": false,
        "items": ["0:3:1562", "0:4"]
      },
      "kind": "module",
      "name": "clap",
      "visibility": "public"
    },
    "0:3:1562": {
      "id": "0:3:1562",
      "inner": {
        "is_crate": false,
        "is_stripped": false,
        "items": ["0:7:1563"]
      },
      "kind": "module",
      "name": "arg_matches",
      "visibility": "crate"
    },
    "0:4": {
      "id": "0:4",
      "inner": {"glob": false, "id": "0:7:1563", "name": "ArgMatches", "source": "arg_matches::ArgMatches"},
      "kind": "import",
      "name": null,
      "visibility": "public"
    },
    "0:7:1563": {
      "id": "0:7:1563",
      "inner": {
        "fields": [],
        "fields_stripped": false,
        "impls": [
          // Auto
          "a:2:9259:2290-0:7:1563", // RefUnwindSafe
          "a:2:3268:211-0:7:1563",  // Send
          "a:2:9258:2291-0:7:1563", // UnwindSafe
          "a:2:3280:220-0:7:1563",  // Sync
          "a:2:3309:1807-0:7:1563", // Unpin
          // Blanket
          "b:2:2871-0:7:1563", // BorrowMut
          "b:2:4001-0:7:1563", // Any
          "b:2:3197-0:7:1563", // Into
          "b:2:3212-0:7:1563", // TryFrom
          "b:2:3207-0:7:1563", // TryInto
          "b:2:2868-0:7:1563", // Borrow
          "b:2:3201-0:7:1563", // From
          "b:5:787-0:7:1563",  // ToOwned
          // Derives
          "0:9",  // Debug
          "0:11", // Clone
          "0:13", // Default
          "0:15", // StructuralPartialEq
          "0:16", // PartialEq
          "0:18", // StructuralEq
          "0:19"  // Eq
        ],
        "struct_type": "tuple"
      },
      "kind": "struct",
      "name": "ArgMatches",
      "visibility": "public"
    }
  },
  "root": "0:0:1564"
}

@rustbot modify labels: -E-needs-mcve

@rustbot rustbot removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Aug 8, 2022
@aDotInTheVoid
Copy link
Member

This is almost certanly due to #99287

searched nightlies: from nightly-2022-07-01 to nightly-2022-08-08
regressed nightly: nightly-2022-07-17
searched commit range: 23e21bd...d5e7f47
regressed commit: d5e7f47

bisected with cargo-bisect-rustc v0.6.4

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 2022-07-01 --script ./script.sh --preserve

script.sh

#!/bin/bash
set -eou
rustdoc -w json -Z unstable-options mcve.rs
python3 ./check.py

mcve.rs

mod arg_matches {
    #[derive(Debug, Clone, Default, PartialEq, Eq)]
    pub struct ArgMatches();
}
pub use arg_matches::ArgMatches;

check.py

import json
with open("doc/mcve.json") as f:
    rdj = json.load(f)
idx = rdj["index"]
for item in idx.values():
    if item["kind"]=="struct" and item["name"] == "ArgMatches":
        impls = item["inner"]["impls"]
        break
if impls:
    exit(0)
else:
    exit(1)

@aDotInTheVoid
Copy link
Member

More minimal (no_core)

#![feature(no_core)]
#![no_core]

mod bar {
    pub struct Baz;
    impl Baz {
        pub fn doit() {}
    }
}

pub use bar::Baz;

Is correct in html
image
and wrong in json

"0:2:1561": {
      "id": "0:2:1561",
      "inner": {
        "fields": [],
        "fields_stripped": false,
        "generics": {"params": [], "where_predicates": []},
        "impls": [],
        "struct_type": "unit"
      },
      "kind": "struct",
      "name": "Baz",
      "visibility": "public"
    }

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 15, 2022
…uillaumeGomez

Rustdoc-Json: Don't remove impls for items imported from private modules

After rust-lang#99287, items in private modules may still be in the json output, if a public import accesses them. To reflect this, items that are imported need to be marked as retained in the `Stripper` pass, so their impls arn't removed by `ImplStripper`.

[More context on zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Populating.20cache.2Eimpls), thanks to @ jyn514 for helping debug this.

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

r? `@GuillaumeGomez`

Fixes rust-lang#100252
Fixes rust-lang#100242
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 15, 2022
…uillaumeGomez

Rustdoc-Json: Don't remove impls for items imported from private modules

After rust-lang#99287, items in private modules may still be in the json output, if a public import accesses them. To reflect this, items that are imported need to be marked as retained in the `Stripper` pass, so their impls arn't removed by `ImplStripper`.

[More context on zulip](https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Populating.20cache.2Eimpls), thanks to @ jyn514 for helping debug this.

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

r? ``@GuillaumeGomez``

Fixes rust-lang#100252
Fixes rust-lang#100242
@bors bors closed this as completed in 44b489f Aug 15, 2022
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. requires-nightly This issue requires a nightly compiler in some way. 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