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.

Sign up for GitHub

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

Jump to bottom

Implement a few more rvalue translation to smir #112717

Merged
merged 3 commits into from
Jul 11, 2023
Merged

Implement a few more rvalue translation to smir #112717

merged 3 commits into from
Jul 11, 2023

Conversation

celinval
Copy link
Contributor

Add the implementation for a few more RValue variants. For now, I simplified the stable version of RValue::Ref by removing the notion of Region.

r? @oli-obk

@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 Jun 16, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2023

This PR changes Stable MIR

cc @oli-obk, @celinval

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 16, 2023

by removing the notion of Region.

one thing I've done for cases like this is to just use a String as the "stable" version, so the data is at least there in some opaque, but printable, manner.

r=me after splitting off the Cargo.toml and extern crate changes into their own commit (and maybe the Stable cleanups that do not add new behaviour, too?)

@bors delegate+

@bors
Copy link
Contributor

bors commented Jun 16, 2023

✌️ @celinval, you can now approve this pull request!

If @oli-obk told you to "r=me" after making some further change, please make that change, then do @bors r=@oli-obk

@celinval
Copy link
Contributor Author

by removing the notion of Region.

one thing I've done for cases like this is to just use a String as the "stable" version, so the data is at least there in some opaque, but printable, manner.

That makes sense. What if I create an actual Opaque type that implements ToString (or any other name you might prefer). Just to differentiate from cases where String is the actual stable version.

r=me after splitting off the Cargo.toml and extern crate changes into their own commit (and maybe the Stable cleanups that do not add new behaviour, too?)

Ok. Just to clarify. Do you want me to split them into separate commit or separate PRs?

@bors delegate+

@oli-obk
Copy link
Contributor

oli-obk commented Jun 17, 2023

What if I create an actual Opaque type that implements ToString (or any other name you might prefer).

Oh heh, that's even better, as then you have an actual type that can even be used in a Stable impls in a way that won't require changing anything but the type and the stable impls in the future when this gets turned into a real conversion.

Do you want me to split them into separate commit or separate PRs?

Just separate commits. It's totally alright to piggyback some cleanup commits into a regular PR

@celinval
Copy link
Contributor Author

@bors r=@oli-obk

@bors
Copy link
Contributor

bors commented Jun 20, 2023

📌 Commit 844dc5ba839ed48ca6c8c82da87b49d240d51f77 has been approved by oli-obk

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 Jun 20, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jun 23, 2023

bors silently ignores approvals that happen before (force-)pushes

@bors r+

@bors
Copy link
Contributor

bors commented Jun 23, 2023

📌 Commit 97e609c has been approved by oli-obk

It is now in the queue for this repository.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 23, 2023

@bors rollup

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 23, 2023
@matthiaskrgr
Rollup merge of rust-lang#112717 - celinval:stable-mir-rvalue-1, r=ol…
71a9222 
…i-obk

Implement a few more rvalue translation to smir

Add the implementation for a few more RValue variants. For now, I simplified the stable version of `RValue::Ref` by removing the notion of Region.

r? `@oli-obk`
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 23, 2023
Closed
@matthiaskrgr
Copy link
Member

@bors r-
might need rebase or something
https://github.com/rust-lang/rust/actions/runs/5358354083/jobs/9720522386#step:25:1244

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 23, 2023
@bors
Copy link
Contributor

bors commented Jul 5, 2023

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

@celinval
Copy link
Contributor Author

celinval commented Jul 7, 2023

@oli-obk I had to solve some conflicts. Do I need another approval?

@oli-obk
Copy link
Contributor

oli-obk commented Jul 8, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2023

📌 Commit 633c022 has been approved by oli-obk

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 8, 2023
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 8, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 8, 2023
@matthiaskrgr
Rollup merge of rust-lang#112717 - celinval:stable-mir-rvalue-1, r=ol…
f521d93 
…i-obk

Implement a few more rvalue translation to smir

Add the implementation for a few more RValue variants. For now, I simplified the stable version of `RValue::Ref` by removing the notion of Region.

r? `@oli-obk`
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 8, 2023
Closed
@matthiaskrgr
Copy link
Member

may need rebase

@bors
Copy link
Contributor

bors commented Jul 8, 2023

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 8, 2023
@celinval
Fix standalone build
44d0fce 
Add extern declarations and optional dependencies to fix build done
directly via `cargo build`.
@celinval
Implement Stable for AssertMessage
6975632
@celinval
Implement a few more rvalue translation to smir
b9f378b 
 - Introduce an Opaque type for adding information that is still
   internal to the compiler.
@celinval
Copy link
Contributor Author

@oli-obk sorry, another conflict...

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2023

@bors r+ rollup

No other smir stuff in the queue, should work this time 😆

@bors
Copy link
Contributor

bors commented Jul 10, 2023

📌 Commit b9f378b has been approved by oli-obk

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 10, 2023
@matthiaskrgr
Rollup merge of rust-lang#112717 - celinval:stable-mir-rvalue-1, r=ol…
f4dfdbf 
…i-obk

Implement a few more rvalue translation to smir

Add the implementation for a few more RValue variants. For now, I simplified the stable version of `RValue::Ref` by removing the notion of Region.

r? `@oli-obk`
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 10, 2023
Closed
@matthiaskrgr
Copy link
Member

@bors rollup=iffy might break clippy tests, lets find out

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2023

@bors rollup

It cannot possibly affect clippy. No one uses the SMIR crate.

@matthiaskrgr matthiaskrgr mentioned this pull request Jul 11, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 11, 2023
@bors
Auto merge of rust-lang#113577 - matthiaskrgr:rollup-vaa83ip, r=matth…
e571544 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#112717 (Implement a few more rvalue translation to smir)
 - rust-lang#113310 (Don't suggest `impl Trait` in path position)
 - rust-lang#113497 (Support explicit 32-bit MIPS ABI for the synthetic object)
 - rust-lang#113560 (Lint against misplaced where-clauses on associated types in traits)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3f73a7d into rust-lang:master Jul 11, 2023
11 checks passed
@rustbot rustbot added this to the 1.73.0 milestone Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

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
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
5 participants
@celinval @rustbot @oli-obk @bors @matthiaskrgr