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

stabilize ptr.is_aligned, move ptr.is_aligned_to to a new feature gate #121948

Merged
merged 1 commit into from
Mar 30, 2024
Merged

stabilize ptr.is_aligned, move ptr.is_aligned_to to a new feature gate #121948

merged 1 commit into from
Mar 30, 2024

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Mar 3, 2024

This is an alternative to #121920

@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2024

r? @scottmcm

rustbot has assigned @scottmcm.
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-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 3, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 3, 2024

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

The Miri subtree was changed

cc @rust-lang/miri

Gankra
Gankra commented Mar 3, 2024
View reviewed changes
compiler/rustc_codegen_gcc/patches/libgccjit12/0001-core-Disable-portable-simd-test.patch
Comment on lines 29 to 31
mod str_lossy;
--
--
2.42.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm text editor settings, not sure if this breaks something

Copy link
Member

Choose a reason for hiding this comment

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

Does it still work with the appropriate git apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

how would i test that? i have no idea how these are used.

Copy link
Member

Choose a reason for hiding this comment

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

just standing in the repo and slapping in git apply compiler/rustc_codegen_gcc/patches/libgccjit12/0001-core-Disable-portable-simd-test.patch

checked and it does, this is gtg. 👍

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@scottmcm scottmcm added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Mar 3, 2024
@scottmcm
Copy link
Member

scottmcm commented Mar 3, 2024

Seems reasonable to me, but takes an FCP, so over to
r? rust-lang/libs-api

@rustbot rustbot assigned m-ou-se and unassigned scottmcm Mar 3, 2024
@scottmcm scottmcm added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 3, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@RalfJung RalfJung mentioned this pull request Mar 3, 2024
Closed
@m-ou-se m-ou-se added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 5, 2024
@m-ou-se
Copy link
Member

m-ou-se commented Mar 5, 2024

FCP to stabilize is_aligned while keeping is_aligned_to unstable:

@rfcbot merge

We might want to remove is_aligned_to, but let's leave that discussion separate from stabilizing is_aligned.

@rfcbot
Copy link

rfcbot commented Mar 5, 2024 •
edited by Amanieu
Loading

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Mar 5, 2024
@m-ou-se m-ou-se removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Mar 5, 2024
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 5, 2024
@rfcbot
Copy link

rfcbot commented Mar 5, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@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 Mar 16, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2024
@bors
Auto merge of rust-lang#121948 - Gankra:stab-align, r=dtolnay
1a06483 
stabilize ptr.is_aligned, move ptr.is_aligned_to to a new feature gate

This is an alternative to rust-lang#121920
@bors
Copy link
Contributor

bors commented Mar 16, 2024

⌛ Testing commit 6f8a40e with merge 1a06483...

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Mar 16, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 16, 2024
@dtolnay
Copy link
Member

dtolnay commented Mar 16, 2024

@rustbot author

@rustbot 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 Mar 16, 2024
@Gankra
Copy link
Contributor Author

Gankra commented Mar 30, 2024

rebased

@workingjubilee
Copy link
Member

Looks fine now!

@bors r=dtolnay

@bors
Copy link
Contributor

bors commented Mar 30, 2024

📌 Commit ea92fae has been approved by dtolnay

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 Mar 30, 2024
@bors
Copy link
Contributor

bors commented Mar 30, 2024

⌛ Testing commit ea92fae with merge 174d07b...

@bors
Copy link
Contributor

bors commented Mar 30, 2024

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing 174d07b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 30, 2024
@bors bors merged commit 174d07b into rust-lang:master Mar 30, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 30, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (174d07b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.8% [-0.8%, -0.7%] 4
Improvements ✅
(secondary)		 -0.5% [-0.7%, -0.3%] 10
All ❌✅ (primary) -0.8% [-0.8%, -0.7%] 4

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.4s -> 669.117s (-0.19%)
Artifact size: 315.77 MiB -> 315.74 MiB (-0.01%)

@WaffleLapkin
Copy link
Member

@Gankra since this merged and you reused the tracking issue number, could you update #96284 to accurately represent what the issue tracks (and tracked)?

@workingjubilee workingjubilee mentioned this pull request Mar 30, 2024
Open
7 tasks
@workingjubilee
Copy link
Member

I fixed up the issue~

@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Apr 4, 2024
@ojeda ojeda mentioned this pull request Apr 5, 2024
Open
90 tasks
@kkysen kkysen mentioned this pull request May 23, 2024
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@workingjubilee workingjubilee workingjubilee left review comments

@dtolnay dtolnay dtolnay approved these changes

Assignees

@dtolnay dtolnay

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.