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 does not escape GAT arguments when it fails to compute the href link for the trait #109488

Closed
fmease opened this issue Mar 22, 2023 · 4 comments · Fixed by #109919
Closed
Assignees
Labels
A-GATs Area: Generic associated types (GATs) A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@fmease
Copy link
Member

fmease commented Mar 22, 2023

Given

pub type A = <S as Tr>::P<Option<i32>>; // `i32` instead of `Option<i32>` would work too but it's not as pronounced

pub struct S;
/*priv*/ trait Tr { type P<T>; }
impl Tr for S { type P<T> = (); }

Rustdoc does not escape the ASCII angle brackets for the generic argument list of the generic associated type projection <S as Tr>::P leading to the following butchered HTML output:

pub type A = <S as Tr>::P>; 

I've noticed this while working on rustdoc integration for AliasKind::Inherent in #109410. It's possible that the refactoring PR #109246 will fix this (need to check) or at least make it more pleasant to fix. Assigning myself for now since I'm gonna fix this otherwise.

@rustbot claim
@rustbot label T-rustdoc A-rustdoc-ui F-generic_associated_types

@fmease fmease added the C-bug Category: This is a bug. label Mar 22, 2023
@rustbot rustbot added A-rustdoc-ui Area: Rustdoc UI (generated HTML) F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 22, 2023
@fmease fmease changed the title Rustdoc does not escape GAT arguments Rustdoc does not escape GAT arguments when it fails to compute the href link for the trait Mar 23, 2023
@jsha
Copy link
Contributor

jsha commented Mar 24, 2023

Copying my comment from #109246:

_ => write!(f, "{}{:#}", assoc.name, assoc.args.print(cx))?,

{:#} is requesting alternate formatting (i.e. plaintext), but in this branch of things, alternate formatting hasn't been requested by the caller. So this is likely emitting < into an HTML context. If you change it to {} in your branch I'm guessing that will fix things.

@jsha
Copy link
Contributor

jsha commented Mar 24, 2023

Here's a live example: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_trait_selection/infer/trait.ObligationEmittingRelation.html#tymethod.register_predicates

As it turns out, this bug does appear to be what's crashing rustdoc in #109246. So I will wind up fixing it. But feel free to investigate and fix if you prefer not to wait.

@jsha
Copy link
Contributor

jsha commented Mar 24, 2023

This wound up being the fix: 236c18f

@fmease
Copy link
Member Author

fmease commented Apr 4, 2023

This wound up being the fix: 236c18f

On master at least I cannot reproduce your fix that involves adding Escape(…) in branch Generic (maybe that only works on your branch). In any case, the assoc.args.print(cx) lines in format.rs are definitely the culprit. I've already fixed it that way in #109410 but I've now also opened a separate PR.

Here's a live example: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_trait_selection/infer/trait.ObligationEmittingRelation.html#tymethod.register_predicates

Thanks for the live example!

aliemjay added a commit to aliemjay/rust that referenced this issue Apr 4, 2023
…=notriddle

rustdoc: escape GAT args in more cases

Fixes rust-lang#109488.

Previously we printed the *un*escaped form of GAT arguments not only when `f.alternate()` was true but *also* when we failed to compute the URL of the trait associated with the type projection, i.e. when `href(…)` returned an `Err(_)`.

In this PR the argument printing logic is entirely separate from the above link resolution code as it should be.
Further, we now only try to compute the URL if the HTML format was requested with `!f.alternate()`. Before, we would sometimes compute the `href` only to throw it away later.
@bors bors closed this as completed in 72e535e Apr 4, 2023
@fmease fmease added the A-GATs Area: Generic associated types (GATs) label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-GATs Area: Generic associated types (GATs) A-rustdoc-ui Area: Rustdoc UI (generated HTML) C-bug Category: This is a bug. F-generic_associated_types `#![feature(generic_associated_types)]` a.k.a. GATs 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.

3 participants