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

Fix is_foreign_item for StableMIR instance #118681

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

celinval
Copy link
Contributor

@celinval celinval commented Dec 6, 2023

Change the implementation of Instance::is_foreign_item to directly query the compiler for the instance def_id instead of incorrectly relying on the conversion to CrateItem. I also added a method to check if the instance has body, since the function already existed and it just wasn't exposed via public APIs. This makes it much cheaper for the user to check if the instance has body.

Background:

r? @ouz-a

Change the implementation of `Instance::is_foreign_item` to directly
query the compiler for the instance `def_id` instead of incorrectly
relying on the conversion to `CrateItem`.

Background:

- In pull rust-lang#118524, I fixed the
  conversion from Instance to CrateItem to avoid the conversion if the
  instance didn't have a body available. This broke the `is_foreign_item`.
This is much cheaper than building a body just for the purpose of
checking if the body exists.
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 6, 2023
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

@ouz-a
Copy link
Contributor

ouz-a commented Dec 6, 2023

lgtm
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Dec 6, 2023

📌 Commit 4a75d18 has been approved by ouz-a

It is now in the queue for this repository.

@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 Dec 6, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 6, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#117981 (Remove deprecated `--check-cfg` syntax)
 - rust-lang#118177 (Suppress warnings in LLVM wrapper when targeting MSVC)
 - rust-lang#118317 (tip for define macro name after `macro_rules!`)
 - rust-lang#118504 (Enforce `must_use` on associated types and RPITITs that have a must-use trait in bounds)
 - rust-lang#118660 (rustc_arena: add `alloc_str`)
 - rust-lang#118681 (Fix is_foreign_item for StableMIR instance )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3c1357c into rust-lang:master Dec 7, 2023
11 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 7, 2023
Rollup merge of rust-lang#118681 - celinval:fix-foreign-item, r=ouz-a

Fix is_foreign_item for StableMIR instance

Change the implementation of `Instance::is_foreign_item` to directly query the compiler for the instance `def_id` instead of incorrectly relying on the conversion to `CrateItem`. I also added a method to check if the instance has body, since the function already existed and it just wasn't exposed via public APIs. This makes it much cheaper for the user to check if the instance has body.

## Background:

- In pull rust-lang#118524, I fixed the conversion from Instance to CrateItem to avoid the conversion if the instance didn't have a body available. This broke the `is_foreign_item`.

r? `@ouz-a`
@rustbot rustbot added this to the 1.76.0 milestone Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants