Skip to content

rustdoc-json: Fix HRTBs for WherePredicate::BoundPredicate#96647

Merged
bors merged 3 commits intorust-lang:masterfrom
Enselic:fix-hrtb-for-wherepredicate
May 18, 2022
Merged

rustdoc-json: Fix HRTBs for WherePredicate::BoundPredicate#96647
bors merged 3 commits intorust-lang:masterfrom
Enselic:fix-hrtb-for-wherepredicate

Conversation

@Enselic
Copy link
Member

@Enselic Enselic commented May 2, 2022 •
edited
Loading

Information about HRTBs are already present for GenericBound:: TraitBound and FunctionPointer. This PR adds HRTB info also to WherePredicate::BoundPredicate.

Use the same field name and type as for the other ones (generic_params: Vec<GenericParamDef>). I have verified that this gives rustdoc JSON clients the data they need and in a format that is easy to work with (see Enselic/public-api#92).

I will be happy to add tests for this change (and bump FORMAT_VERSION which I just realized I forgot), but it is always nice to get one round of feedback first, so that I don't put a lot of effort into tests that then have to be discarded.

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

@rust-highfive
Copy link
Contributor

rust-highfive commented May 2, 2022

rustdoc-json-types is a public (although nightly-only) API. If possible, consider changing src/librustdoc/json/conversions.rs; otherwise, make sure you bump the FORMAT_VERSION constant.

cc @CraftSpider,@aDotInTheVoid

@rustbot rustbot added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-json Area: Rustdoc JSON backend labels May 2, 2022
@rust-highfive
Copy link
Contributor

rust-highfive commented May 2, 2022

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2022
@Enselic Enselic mentioned this pull request May 2, 2022
Merged
@aDotInTheVoid
Copy link
Member

aDotInTheVoid commented May 2, 2022

Overall looks good, but as you say, it needs a format version bump, and tests. Idealy tests not just for the new field, but for the pre-existing ones you commented on, as I don't think we have anything covers does them.

@Enselic
Copy link
Member Author

Enselic commented May 3, 2022

Thank you for taking an early look. Next, I will add tests for all HRTB fields.

@rustbot label +S-waiting-on-author -S-waiting-on-review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2022
@Enselic Enselic force-pushed the fix-hrtb-for-wherepredicate branch from 7c9b52a to 774b525 Compare May 4, 2022 19:06
@Enselic
Copy link
Member Author

Enselic commented May 4, 2022 •
edited
Loading

Tests added and FORMAT_VERSION bumped. So this is ready to be merged after review as far as I know. But we need to keep an eye on #96680 which also bumps FORMAT_VERSION to the same number.

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2022
@Enselic
Copy link
Member Author

Enselic commented May 5, 2022

r? @CraftSpider

@Enselic
Copy link
Member Author

Enselic commented May 11, 2022

Is @CraftSpider the right reviewer? Or should I set you instead @aDotInTheVoid ?

@aDotInTheVoid
Copy link
Member

aDotInTheVoid commented May 11, 2022

I don't have bors privileges. Craftspider generally reviews rustdoc-json stuff.

@Enselic Enselic mentioned this pull request May 12, 2022
Closed
@CraftSpider
Copy link
Contributor

CraftSpider commented May 17, 2022

Yes, thanks for the patience. I moved recently, and sometimes take a bit of time on reviews. I'll get this reviewed tonight.

@CraftSpider
Copy link
Contributor

CraftSpider commented May 17, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented May 17, 2022

📌 Commit 774b525 has been approved by CraftSpider

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 17, 2022
@Enselic Enselic mentioned this pull request May 17, 2022
Merged
@aDotInTheVoid aDotInTheVoid mentioned this pull request May 17, 2022
Closed
@JohnTitor JohnTitor mentioned this pull request May 17, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2022
@bors
Auto merge of rust-lang#97126 - JohnTitor:rollup-7gdt2aw, r=JohnTitor
77972d2 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#96647 (rustdoc-json: Fix HRTBs for WherePredicate::BoundPredicate)
 - rust-lang#96651 (Omit unnecessary help to add `#[cfg(test)]` when already annotated)
 - rust-lang#96761 (rustdoc: don't build `rayon` for non-windows targets)
 - rust-lang#97096 (Types with reachable constructors are reachable)
 - rust-lang#97097 (Add tmm_reg clobbers)
 - rust-lang#97113 (Search GUI fixes)
 - rust-lang#97116 (interpret/validity: reject references to uninhabited types)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ce49ce2 into rust-lang:master May 18, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 18, 2022
@Enselic Enselic deleted the fix-hrtb-for-wherepredicate branch May 26, 2022 19:55
@Enselic Enselic restored the fix-hrtb-for-wherepredicate branch May 26, 2022 19:55
@Enselic Enselic deleted the fix-hrtb-for-wherepredicate branch May 26, 2022 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@CraftSpider CraftSpider CraftSpider approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@CraftSpider CraftSpider

Labels

A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.63.0

Development

Successfully merging this pull request may close these issues.

7 participants

@Enselic @rust-highfive @aDotInTheVoid @CraftSpider @bors @Mark-Simulacrum @rustbot