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

interpret: refactor function call handling to be better-abstracted #128687

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 5, 2024

Add a new function init_stack_frame that pushes a stack frame and passes the arguments, and use that basically everywhere that the raw underlying push_stack_frame used to be called. This splits the previous monster function eval_fn_call into two parts: figuring out the MIR to call and the arguments to pass, and then actually setting up the stack frame.

Also re-organize the files a bit:

  • The previous terminator.rs is split into a new call.rs with all the argument-passing logic, and the rest goes into step.rs where the other main dispatcher functions already live (in particular, eval_statement).
  • All the stack frame handling from eval_context.rs is moved to a new stack.rs.

@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2024

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2024

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

@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 Aug 5, 2024
@RalfJung RalfJung force-pushed the interpret-call-refactor branch from b3caa1b to 498d687 Compare August 5, 2024 15:55
@rust-log-analyzer

This comment has been minimized.

@RalfJung RalfJung force-pushed the interpret-call-refactor branch 2 times, most recently from 721ee9b to b02aa4f Compare August 5, 2024 16:25
@bors
Copy link
Contributor

bors commented Aug 6, 2024

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

@RalfJung RalfJung force-pushed the interpret-call-refactor branch from 57f9d09 to 5783e73 Compare August 6, 2024 09:08
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

The changes generally look good, but I have a few nits/questions

compiler/rustc_const_eval/src/interpret/step.rs Outdated Show resolved Hide resolved
compiler/rustc_const_eval/src/interpret/call.rs Outdated Show resolved Hide resolved
compiler/rustc_const_eval/src/interpret/call.rs Outdated Show resolved Hide resolved
compiler/rustc_const_eval/src/interpret/call.rs Outdated Show resolved Hide resolved
compiler/rustc_const_eval/src/interpret/stack.rs Outdated Show resolved Hide resolved
@WaffleLapkin
Copy link
Member

r? WaffleLapkin

@rustbot rustbot assigned WaffleLapkin and unassigned wesleywiser Aug 6, 2024
@RalfJung RalfJung force-pushed the interpret-call-refactor branch from ca0af71 to 1c2705c Compare August 6, 2024 11:49
Copy link
Member

@WaffleLapkin WaffleLapkin left a comment

Choose a reason for hiding this comment

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

r=me with green CI

@RalfJung
Copy link
Member Author

RalfJung commented Aug 6, 2024

@bors r=WaffleLapkin

@bors
Copy link
Contributor

bors commented Aug 6, 2024

📌 Commit 1c2705c has been approved by WaffleLapkin

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 Aug 6, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 6, 2024
…r=WaffleLapkin

interpret: refactor function call handling to be better-abstracted

Add a new function `init_stack_frame` that pushes a stack frame and passes the arguments, and use that basically everywhere that the raw underlying `push_stack_frame` used to be called. This splits the previous monster function `eval_fn_call` into two parts: figuring out the MIR to call and the arguments to pass, and then actually setting up the stack frame.

Also re-organize the files a bit:
- The previous `terminator.rs` is split into a new `call.rs` with all the argument-passing logic, and the rest goes into `step.rs` where the other main dispatcher functions already live (in particular, `eval_statement`).
- All the stack frame handling from `eval_context.rs` is moved to a new `stack.rs`.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128273 (Improve `Ord` violation help)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 6, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128273 (Improve `Ord` violation help)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#124944 (On trait bound mismatch, detect multiple crate versions in dep tree)
 - rust-lang#125048 (PinCoerceUnsized trait into core)
 - rust-lang#128406 (implement BufReader::peek)
 - rust-lang#128539 (Forbid unused unsafe in vxworks-specific std modules)
 - rust-lang#128687 (interpret: refactor function call handling to be better-abstracted)
 - rust-lang#128692 (Add a triagebot mention for `library/Cargo.lock`)
 - rust-lang#128710 (Don't ICE when getting an input file name's stem fails)
 - rust-lang#128718 (Consider `cfg_attr` checked by `CheckAttrVisitor`)
 - rust-lang#128751 (std::thread: set_name implementation proposal for vxWorks.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 52c365b into rust-lang:master Aug 7, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
Rollup merge of rust-lang#128687 - RalfJung:interpret-call-refactor, r=WaffleLapkin

interpret: refactor function call handling to be better-abstracted

Add a new function `init_stack_frame` that pushes a stack frame and passes the arguments, and use that basically everywhere that the raw underlying `push_stack_frame` used to be called. This splits the previous monster function `eval_fn_call` into two parts: figuring out the MIR to call and the arguments to pass, and then actually setting up the stack frame.

Also re-organize the files a bit:
- The previous `terminator.rs` is split into a new `call.rs` with all the argument-passing logic, and the rest goes into `step.rs` where the other main dispatcher functions already live (in particular, `eval_statement`).
- All the stack frame handling from `eval_context.rs` is moved to a new `stack.rs`.
@RalfJung RalfJung deleted the interpret-call-refactor branch August 7, 2024 09:27
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.

6 participants