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] Foreign items that are only re-exported are no longer included in the index #99513

Open
Urgau opened this issue Jul 20, 2022 · 3 comments
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

@Urgau
Copy link
Member

Urgau commented Jul 20, 2022

PR #99287 changed the behavior of the JSON output of rustdoc to no longer do inlining of re-export as it was causing many problems. However the new behavior do not account for foreign items that are only re-exported as they are no longer included in the index.

We should fix this by re-introducing them in the index. But how ? I tried hacking around by doing partial-inlining but this will just re-introduce the problems we has before. I someone wants to build on what I did here is the code:

diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
index ce10ca9aa3d..5274a211986 100644
--- a/src/librustdoc/clean/inline.rs
+++ b/src/librustdoc/clean/inline.rs
@@ -153,7 +153,18 @@ pub(crate) fn try_inline_glob(
     match res {
         Res::Def(DefKind::Mod, did) => {
             let m = build_module(cx, did, visited);
-            Some(m.items)
+            if cx.output_format.is_json() {
+                let m = clean::ModuleItem(m);
+                let m = clean::Item::from_def_id_and_parts(
+                    did,
+                    None,
+                    m,
+                    cx,
+                );
+                Some(vec![m])
+            } else {
+                Some(m.items)
+            }
         }
         // glob imports on things like enums aren't inlined even for local exports, so just bail
         _ => None,
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 9865601da5f..53a3246ae87 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -2152,8 +2152,7 @@ fn clean_use_statement<'tcx>(
     // forcefully don't inline if this is not public or if the
     // #[doc(no_inline)] attribute is present.
     // Don't inline doc(hidden) imports so they can be stripped at a later stage.
-    let mut denied = cx.output_format.is_json()
-        || !(visibility.is_public()
+    let mut denied = !(visibility.is_public()
             || (cx.render_options.document_private && is_visible_from_parent_mod))
         || pub_underscore
         || attrs.iter().any(|a| {
@@ -2170,11 +2169,16 @@ fn clean_use_statement<'tcx>(
     // Also check whether imports were asked to be inlined, in case we're trying to re-export a
     // crate in Rust 2018+
     let path = path.clean(cx);
+    let mut inner_items = None;
     let inner = if kind == hir::UseKind::Glob {
         if !denied {
             let mut visited = FxHashSet::default();
             if let Some(items) = inline::try_inline_glob(cx, path.res, &mut visited) {
-                return items;
+                if cx.output_format.is_json() {
+                    inner_items = Some(items);
+                } else {
+                    return items;
+                }
             }
         }
         Import::new_glob(resolve_use_source(cx, path), true)
@@ -2204,16 +2208,25 @@ fn clean_use_statement<'tcx>(
                 items.push(Item::from_def_id_and_parts(
                     import_def_id,
                     None,
-                    ImportItem(Import::new_simple(name, resolve_use_source(cx, path), false)),
+                    ImportItem(Import::new_simple(name, resolve_use_source(cx, path.clone()), false)),
                     cx,
                 ));
-                return items;
+                if cx.output_format.is_json() {
+                    inner_items = Some(items);
+                } else {
+                    return items;
+                }
             }
         }
         Import::new_simple(name, resolve_use_source(cx, path), true)
     };
 
-    vec![Item::from_def_id_and_parts(import.def_id.to_def_id(), None, ImportItem(inner), cx)]
+    if let Some(mut inner_items) = inner_items {
+        inner_items.push(Item::from_def_id_and_parts(import.def_id.to_def_id(), None, ImportItem(inner), cx));
+        inner_items
+    } else {
+        vec![Item::from_def_id_and_parts(import.def_id.to_def_id(), None, ImportItem(inner), cx)]
+    }
 }
 
 fn clean_maybe_renamed_foreign_item<'tcx>(

cc @aDotInTheVoid @CraftSpider @Enselic
@rustbot labels +A-rustdoc-json +T-rustdoc +requires-nightly

@Urgau Urgau added the C-bug Category: This is a bug. label Jul 20, 2022
@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend 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. labels Jul 20, 2022
@Enselic
Copy link
Member

Enselic commented Jul 20, 2022

Could you exemplify what you mean please? This example code is a re-export of a foreign item, right? And that works (with mentioned limitations further down): #99287 (comment)

@Urgau
Copy link
Member Author

Urgau commented Jul 21, 2022

Could you exemplify what you mean please?

Sure here is real-world case I stumble across: https://github.com/mrhooray/crc-rs/blob/4618281222223265e24772ce89f54bff45bda03c/src/lib.rs#L34 in this case there is a pub use crc_catalog::*; where crc_catalog is an external crate, in the JSON there is glob Import for this line that reference crc_catalog with an Id but there is no corresponding Id in the index! Every Id should have a corresponding item in the index but because none of the item are used explicitly in crc rustdoc doesn't add them to the index. That's the problem!

Minimal reproducer with your bash script
# Create a library with a trait with one associated function
echo '
pub struct MyStruct;
' > a.rs

# Create another library that depends on the first library
echo '
pub use a::*;
' > b.rs

toolchain="nightly"

mkdir $toolchain

# Compile the library
rustc +$toolchain --edition=2021 \
        --crate-type rlib -o $toolchain/liba.rlib a.rs

# Generate rustdoc JSON for both libraries
rustdoc +$toolchain --edition=2021 \
        --extern a=$toolchain/liba.rlib \
        -Z unstable-options --output-format json \
        -o $toolchain b.rs

# See if rustdoc JSON for `trait_a_fn` was generated
grep --silent MyStruct $toolchain/b.json && echo "rustdoc JSON for 'MyStruct' was generated by '$toolchain'"

This example code is a re-export of a foreign item, right? And that works (with mentioned limitations further down): #99287 (comment)

Yeah, I may have spoken to soon, if the trait A has items that reference trait_in_fn with an id but that id isn't included then it's the same problem but if their are no items it's a different issue (if it's an issue).

@Enselic
Copy link
Member

Enselic commented Jul 21, 2022

There indeed seems to be a bug when wildcards are used. Here is a reproducer. Copy-paste to your bash shell:

# Create a library with a trait with one associated function
echo '
#![feature(no_core)]
#![no_core]
#![no_std]
pub trait A {
    fn trait_a_fn();
}
' > a.rs

# Create another library that depends on the first library
echo '
#![feature(no_core)]
#![no_core]
#![no_std]
pub use a::*;
' > b.rs

# Compile the library
rustc +nightly --edition=2021 --crate-type rlib a.rs

# Generate rustdoc JSON for both libraries
rustdoc +nightly --edition=2021 --extern a=liba.rlib -Z unstable-options --output-format json b.rs

# Missing "kind": "trait" item
cat doc/b.json | python3 -m json.tool

There is no "kind": "trait" in the JSON output. But if you just change pub use a::* to pub use a::A, then it appears. But it should appear also with the wildcard, I think.

Urgau added a commit to Urgau/rust that referenced this issue Oct 29, 2022
Introduce a new `external_index` that only contains a `ExternalItem`
struct from the previous `ItemSummary`. This "new" index only contains
external item and is only here so that IDs continue to referring to
an "item".

A previous version of this patch also removed `path` but at least until
rust-lang#99513 is fixed it is
unfortunately still required (as a workaround).
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

No branches or pull requests

3 participants