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

changes old intrinsic declaration to new declaration #133106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BLANKatGITHUB
Copy link
Contributor

This pr is for issue #132735

It changes old extern "intrinsic" code block with new declaration.

There are other blocks that use old declaration but as the changes needed in single block is quite large I do them in parts

@rustbot
Copy link
Collaborator

rustbot commented Nov 16, 2024

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
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 rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 16, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 16, 2024

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @rust-lang/wg-const-eval

@BLANKatGITHUB
Copy link
Contributor Author

Hello?

@RalfJung
Copy link
Member

It's only been 3 days. Reviewers are largely volunteers doing this next to their dayjobs and regular life. Please have a bit of patience. :)

Quoting from the comment posted by the bot above (emphasis mine):
"They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer."

@BLANKatGITHUB
Copy link
Contributor Author

@RalfJung I just wanted to check, I didn't wanted to be rude. sorry if I was

@RalfJung
Copy link
Member

Looks good, thanks. :)

@bors r+

@bors
Copy link
Contributor

bors commented Nov 23, 2024

📌 Commit 4250bfe has been approved by RalfJung

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 Nov 23, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 23, 2024
changes old intrinsic declaration to new declaration

This pr is for issue rust-lang#132735

It changes old `extern "intrinsic"` code block with new declaration.

There are other blocks that use old declaration but as the changes needed in single block is quite large I do them in parts
@jieyouxu
Copy link
Member

Failed in rollup, needs to rebless a UI test.

2024-11-23T12:41:36.0603872Z ---- [ui] tests/ui/intrinsics/reify-intrinsic.rs stdout ----
2024-11-23T12:41:36.0606692Z Saved the actual stderr to "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/intrinsics/reify-intrinsic/reify-intrinsic.stderr"
2024-11-23T12:41:36.0607757Z diff of stderr:
2024-11-23T12:41:36.0607954Z 
2024-11-23T12:41:36.0608235Z 22	   |         ^^^^^^^^^^^^^^^^^^^^^^^^^ cannot coerce intrinsics to function pointers
2024-11-23T12:41:36.0608765Z 23	   |
2024-11-23T12:41:36.0609346Z 24	   = note: expected fn pointer `unsafe extern "rust-intrinsic" fn(_) -> _`
2024-11-23T12:41:36.0610229Z -	                 found fn item `unsafe extern "rust-intrinsic" fn(_) -> _ {floorf32}`
2024-11-23T12:41:36.0610987Z +	                 found fn item `unsafe fn(_) -> _ {floorf32}`
2024-11-23T12:41:36.0611442Z 26	
2024-11-23T12:41:36.0611756Z 27	error: aborting due to 3 previous errors

@bors r-

@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 Nov 23, 2024
@BLANKatGITHUB
Copy link
Contributor Author

umm @jieyouxu is there a problem?
Because I ran the tests/ui/intrinstics in my mchine and all the tests passed

@jieyouxu
Copy link
Member

jieyouxu commented Nov 23, 2024

Hi @BLANKatGITHUB, if you rebase against latest master, you should be able to observe the failure. I was not able to repro the failure locally (x86_64-pc-windows-msvc) with your branch immediately, but I am able to repro the failure when I rebased against latest master.

@BLANKatGITHUB
Copy link
Contributor Author

Well I confirmed it and it indeed fails on latest master branch.
can you pls give a little insight on what is causing it and how can I resolve it

@RalfJung
Copy link
Member

You have to rebase (see https://rustc-dev-guide.rust-lang.org/git.html#rebasing-and-conflicts), and then run ./x test --bless -- intrinsics. That will update the test reference files.

sql.txt Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Something went wrong here, this file shouldn't be in the PR.

Please always double-check your commits before pushing them!

Copy link
Member

Choose a reason for hiding this comment

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

Now you have a commit adding the sql.txt file and another commit removing it. Please squash those into a single commit (or squash the entire PR into a single commit if you prefer).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I know sorry for that

blesses tests/ui/intrinsics

blesses tests/ui/intrinsics
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library 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