Pass FnAbi to find_mir_or_eval_fn by tiif · Pull Request #133103 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation56 Commits1 Checks6 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

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

Nov 16, 2024

Some changes occurred to the CTFE machinery

cc @rust-lang/wg-const-eval

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

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

Nov 24, 2024

@RalfJung There are some refactorings needed to remove ExternAbi from find_mir_or_eval_fn. Specifically, all functions that call this.check_shim will need to be updated. Could I confirm if this direction is what we are going for?

This comment has been minimized.

Specifically, all functions that call this.check_shim will need to be updated.

Yeah, they should pass fn_abi to check_shim, and all the ExternAbi::C { unwind: false } become abi::Conv::C.

This comment has been minimized.

Interesting... for the same test, running on Linux will pass, but running on Windows will fail because of incompatible ABI.

For future reference:

./x.py test miri --test-args tests/pass/align.rs --target x86_64-pc-windows-gnu failed with calling a function with ABI X86Stdcall using caller ABI C
./x.py test miri --test-args tests/pass/align.rs --target x86_64-unknown-linux-gnu pass.

This means that for the exact same function, whether we should use Conv::C or Conv::X86Stdcall depends on the target we are running? Or maybe I just did something bad somewhere...

EDIT: Ok nevermind, it's just I used Conv::X86Stdcall when it supposed to be Conv::C, even for the same tests, it hits different path for Windows and Linux.

This means that for the exact same function, whether we should use Conv::C or Conv::X86Stdcall depends on the target we are running? Or maybe I just did something bad somewhere...

Yes that makes sense, Windows has some special rules here.

I might continue to fix some cosmetic issues such as stray new lines, but this PR is ready for review

@rustbot ready

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

Dec 14, 2024

Choose a reason for hiding this comment

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

Looks pretty good. :) Just some minor comments.

Comment on lines 936 to 938

if fn_abi.conv != exp_abi {
throw_ub_format!(
"calling a function with ABI {} using caller ABI {}",

Choose a reason for hiding this comment

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

Just use debug formatting for the error for now. We do the same in interpret/call.rs already.

The proper fix here would be a impl Display for Conv somewhere near this. If you want you can do that in a future PR. :)

Choose a reason for hiding this comment

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

Sure, I can open one after this.

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

Dec 15, 2024

jieyouxu added a commit to jieyouxu/rust that referenced this pull request

Dec 17, 2024

Pass FnAbi to find_mir_or_eval_fn

rust-lang/miri#4013 needs information from FnAbi, hence it is passed to find_mir_or_eval_fn.

r? @RalfJung

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

Dec 17, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Dec 18, 2024

…iaskrgr

Rollup of 7 pull requests

Successful merges:

rust-lang#133265 (Add a range argument to vec.extract_if)
rust-lang#133801 (Promote powerpc64le-unknown-linux-musl to tier 2 with host tools)
rust-lang#134323 (coverage: Dismantle map_data.rs by moving its responsibilities elsewhere)
rust-lang#134378 (An octuple of polonius fact generation cleanups)
rust-lang#134408 (Regression test for RPIT inheriting lifetime from projection)
rust-lang#134423 (bootstrap: use specific-purpose ui test path for test_valid self-test)
rust-lang#134426 (Fix typo in uint_macros.rs)

Failed merges:

rust-lang#133103 (Pass FnAbi to find_mir_or_eval_fn)

r? @ghost @rustbot modify labels: rollup

I will need some time to figure out how to resolve the merge conflict, and will request for r+ again after resolving it.

Hmm this is weird, the import is valid and I didn't change anything from attr, but got this error:

error[E0432]: unresolved import rustc_ast::UnsafeBinderCastKind --> compiler/rustc_hir/src/hir.rs:14:57 | 14 | ImplPolarity, IsAuto, Movability, Mutability, UnOp, UnsafeBinderCastKind, | ^^^^^^^^^^^^^^^^^^^^ no UnsafeBinderCastKind in the root

error[E0432]: unresolved import rustc_ast::attr::AttributeExt --> compiler/rustc_hir/src/lang_items.rs:10:5 | 10 | use rustc_ast::attr::AttributeExt; | ^^^^^^^^^^^^^^^^^------------ | | | | | help: a similar name exists in the module: Attribute | no AttributeExt in attr

For more information about this error, try rustc --explain E0432.

@oli-obk since you reviewed #134381, do you know what's going on here and how to resolve this?

That's weird. Sounds like a cache issue where it doesn't think a crate has changed when it acrually has. Worst case try x clean

This comment has been minimized.

📌 Commit fd8b983 has been approved by RalfJung

It is now in the queue for this repository.

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

Dec 19, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request

Dec 20, 2024

Rollup of 6 pull requests

Successful merges:

rust-lang#126118 (docs: Mention spare_capacity_mut() in Vec::set_len)
rust-lang#132830 (Rename elem_offset to element_offset)
rust-lang#133103 (Pass FnAbi to find_mir_or_eval_fn)
rust-lang#134321 (Hide = _ as associated constant value inside impl blocks)
rust-lang#134518 (fix typos in the example code in the doc comments of Ipv4Addr::from_bits(), Ipv6Addr::from_bits() & Ipv6Addr::to_bits())
rust-lang#134521 (Arbitrary self types v2: roll loop.)

r? @ghost @rustbot modify labels: rollup

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Dec 20, 2024

Rollup merge of rust-lang#133103 - tiif:fnabi, r=RalfJung

Pass FnAbi to find_mir_or_eval_fn

rust-lang/miri#4013 needs information from FnAbi, hence it is passed to find_mir_or_eval_fn.

r? @RalfJung

tiif mentioned this pull request

Jan 21, 2025

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.