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

Add basic trait impls for f16 and f128 #123085

Merged
merged 1 commit into from
Mar 31, 2024
Merged

Add basic trait impls for f16 and f128 #123085

merged 1 commit into from
Mar 31, 2024

Conversation

tgross35
Copy link
Contributor

Split off part of #122470 so the compiler doesn't ICE because it expects primitives to have some minimal traits.

Fixes #123074

@tgross35 tgross35 marked this pull request as ready for review March 26, 2024 09:24
@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2024

r? @joboet

rustbot has assigned @joboet.
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 Mar 26, 2024
@tgross35
Copy link
Contributor Author

@rustbot label +T-libs-api +F-f16_and_f128 -T-libs
r? @Amanieu

@rustbot rustbot added F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 26, 2024
@rustbot rustbot assigned Amanieu and unassigned joboet Mar 26, 2024
@tgross35 tgross35 mentioned this pull request Mar 26, 2024
Closed
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@tgross35
Copy link
Contributor Author

Wow, I am really stuck in some circular dependencies if this can't even go through

@rustbot blocked

I think #123087 should let this continue

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2024
@compiler-errors
Copy link
Member

You probably need to #[cfg(not(bootstrap))] some of this code.

@compiler-errors
Copy link
Member

Ah, nvm, it's clippy I guess.

@tgross35
Copy link
Contributor Author

Yeah lol, it looks like I need to kind of inch my way out of circular dependencies

@tgross35 tgross35 force-pushed the f16-f128-step4.0-libs-basic-impls branch 2 times, most recently from 8c7d1d3 to cd1e659 Compare March 27, 2024 00:50
@tgross35
Copy link
Contributor Author

This should be good with #123087 unless there is another branch of Clippy that gets exercised here

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Mar 27, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@tgross35 tgross35 force-pushed the f16-f128-step4.0-libs-basic-impls branch from cd1e659 to fb9a93f Compare March 27, 2024 01:07
@tgross35
Copy link
Contributor Author

Oh huh, I guess clippy tests gets run with beta so my change made no difference. Tough to tell exactly where it is coming from but I'll see if I can get away just cfging out Default makes a difference, since that should be the only place a literal pops up.

@bors
Copy link
Contributor

bors commented Mar 28, 2024

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

@tgross35
Add basic trait impls for f16 and f128
d7d5fc9 
Split off part of <rust-lang#122470> so the
compiler doesn't ICE because it expects primitives to have some minimal
traits.

Fixes <rust-lang#123074>
@tgross35 tgross35 force-pushed the f16-f128-step4.0-libs-basic-impls branch from fb9a93f to d7d5fc9 Compare March 28, 2024 19:02
@Amanieu
Copy link
Member

Amanieu commented Mar 30, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 30, 2024

📌 Commit d7d5fc9 has been approved by Amanieu

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

bors commented Mar 30, 2024

⌛ Testing commit d7d5fc9 with merge 5da1a1b...

@bors
Copy link
Contributor

bors commented Mar 31, 2024

☀️ Test successful - checks-actions
Approved by: Amanieu
Pushing 5da1a1b to master...

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

Finished benchmarking commit (5da1a1b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -3.6% [-4.3%, -2.5%] 4
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 669.242s -> 668.99s (-0.04%)
Artifact size: 315.71 MiB -> 315.70 MiB (-0.01%)

@tgross35 tgross35 deleted the f16-f128-step4.0-libs-basic-impls branch March 31, 2024 16:36
tgross35 added a commit to tgross35/rust that referenced this pull request Apr 2, 2024
@tgross35
Put basic impls for f16 and f128 behind cfg(not(bootstrap))
049a917 
We will lose `f16` and `f128` in the beta compiler after the revert for
<rust-lang#123282> lands. Change what was
added in <rust-lang#123085> to be behind
`#[cfg(not(bootstrap))]` to account for this.
@tgross35 tgross35 mentioned this pull request Apr 2, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2024
@bors
Auto merge of rust-lang#123390 - tgross35:f16-f128-libs-basic-impls-b…
ceab612 
…ootstrap, r=jhpratt

Put basic impls for f16 and f128 behind cfg(not(bootstrap))

We will lose `f16` and `f128` in the beta compiler after the revert for <rust-lang#123282> lands. Change what was added in <rust-lang#123085> to be behind `#[cfg(not(bootstrap))]` to account for this.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 4, 2024
@bors
Auto merge of #123390 - tgross35:f16-f128-libs-basic-impls-bootstrap,…
607cf8c 
… r=jhpratt

Put basic impls for f16 and f128 behind cfg(not(bootstrap))

We will lose `f16` and `f128` in the beta compiler after the revert for <rust-lang/rust#123282> lands. Change what was added in <rust-lang/rust#123085> to be behind `#[cfg(not(bootstrap))]` to account for this.
@tgross35 tgross35 mentioned this pull request Apr 7, 2024
Open
85 tasks
kjetilkjeka pushed a commit to kjetilkjeka/rust that referenced this pull request Apr 15, 2024
@tgross35
Put basic impls for f16 and f128 behind cfg(not(bootstrap))
a74a047 
We will lose `f16` and `f128` in the beta compiler after the revert for
<rust-lang#123282> lands. Change what was
added in <rust-lang#123085> to be behind
`#[cfg(not(bootstrap))]` to account for this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Amanieu Amanieu

Labels
F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` merged-by-bors This PR was explicitly merged by bors. 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.

ICE related to f16 / f128
8 participants
@tgross35 @rustbot @rust-log-analyzer @compiler-errors @bors @Amanieu @rust-timer @joboet