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

Add abi field to Method #81502

Merged
merged 2 commits into from
Feb 7, 2021
Merged

Add abi field to Method #81502

merged 2 commits into from
Feb 7, 2021

Conversation

CraftSpider
Copy link
Contributor

Also bumps version and adds a test (Will conflict with #81500, whichever is merged first)

Rationale: It's possible for methods to have an ABI. This should be exposed in the JSON.

@rust-highfive
Copy link
Collaborator

r? @ollie27

(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 Jan 29, 2021
@CraftSpider
Copy link
Contributor Author

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

@rustbot rustbot added A-rustdoc-json Area: Rustdoc JSON backend T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 29, 2021
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but this will conflict with the Union change.

@jyn514
Copy link
Member

jyn514 commented Feb 4, 2021

Maybe we should consider 'bundling' the changes together somehow so the version doesn't change as often? This isn't a breaking change either, it seems a shame - we could use 3.1 or something maybe?

@CraftSpider
Copy link
Contributor Author

I mean, as it's non-breaking, I think it might be okay if there's a way to get them merged in the same nightly, or such? I guess I could just make a new branch with both of these and PR it, so they merge together

@bors
Copy link
Contributor

bors commented Feb 5, 2021

☔ The latest upstream changes (presumably #81784) made this pull request unmergeable. Please resolve the merge conflicts.

@CraftSpider
Copy link
Contributor Author

Rebased. Didn't update version number yet, as I'm not sure if we want to for a non-breaking change?

@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error: expected identifier, found `<<`
   --> src/librustdoc/json/conversions.rs:437:1
    |
437 | <<<<<<< HEAD
    | ^^ expected identifier
error[E0432]: unresolved import `crate::json::conversions::from_def_id`
  --> src/librustdoc/json/mod.rs:27:5
   |
27 | use crate::json::conversions::from_def_id;
27 | use crate::json::conversions::from_def_id;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `from_def_id` in `json::conversions`

error[E0277]: the trait bound `rustdoc_json_types::Trait: From<types::Trait>` is not satisfied
   --> src/librustdoc/json/mod.rs:111:63
    |
111 | ...                   inner: types::ItemEnum::TraitItem(trait_item.clone().into()),
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `From<types::Trait>` is not implemented for `rustdoc_json_types::Trait`
    |
    = note: required because of the requirements on the impl of `Into<rustdoc_json_types::Trait>` for `types::Trait`

error[E0599]: no method named `convert_item` found for mutable reference `&mut JsonRenderer<'tcx>` in the current scope
   --> src/librustdoc/json/mod.rs:160:42
    |
160 |         if let Some(mut new_item) = self.convert_item(item) {
    |                                          ^^^^^^^^^^^^ method not found in `&mut JsonRenderer<'tcx>`

error[E0277]: the trait bound `rustdoc_json_types::ItemKind: From<ItemType>` is not satisfied
   --> src/librustdoc/json/mod.rs:225:91
    |
225 |                         types::ItemSummary { crate_id: k.krate.as_u32(), path, kind: kind.into() },
    |                                                                                           ^^^^ the trait `From<ItemType>` is not implemented for `rustdoc_json_types::ItemKind`
    |
    = note: required because of the requirements on the impl of `Into<rustdoc_json_types::ItemKind>` for `ItemType`
error: aborting due to 5 previous errors

Some errors have detailed explanations: E0277, E0432, E0599.
For more information about an error, try `rustc --explain E0277`.

@jyn514
Copy link
Member

jyn514 commented Feb 6, 2021

rust-lang/rfcs#2963 (comment) says the main reason for a format-version is to avoid breaking changes, so I agree I don't think we need to bump it for a non-breaking change.

@bors r+

@bors
Copy link
Contributor

bors commented Feb 6, 2021

📌 Commit ac75faf has been approved by jyn514

@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 Feb 6, 2021
@bors
Copy link
Contributor

bors commented Feb 7, 2021

⌛ Testing commit ac75faf with merge ae00b62...

@bors
Copy link
Contributor

bors commented Feb 7, 2021

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing ae00b62 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 7, 2021
@bors bors merged commit ae00b62 into rust-lang:master Feb 7, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 7, 2021
@CraftSpider CraftSpider deleted the method-abi branch February 7, 2021 21:35
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 merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

7 participants