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

Change f16 and f128 clippy stubs to be nonpanicking #123087

Merged
merged 1 commit into from
Mar 27, 2024
Merged

Change f16 and f128 clippy stubs to be nonpanicking #123087

merged 1 commit into from
Mar 27, 2024

Conversation

tgross35
Copy link
Contributor

It turns out there is a bit of a circular dependency - I cannot add anything to core because Clippy fails, and I can't actually add correct Clippy implementations without new implementations from core.

Change some of the Clippy stubs from unimplemented! to success values and leave a FIXME in their place to mitigate this.

Fixes #122587

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

rustbot commented Mar 26, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2024
@tgross35
Copy link
Contributor Author

@rustbot label +A-clippy

r? clippy

@rustbot rustbot added the A-clippy Area: Clippy label Mar 26, 2024
@rustbot rustbot assigned xFrednet and unassigned Mark-Simulacrum Mar 26, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Mar 26, 2024 •
edited
Loading

Cc @matthiaskrgr since you were looking at this recently

Hey looks like you're on the clippy team too if you want to review :)

@tgross35 tgross35 mentioned this pull request Mar 26, 2024
Merged
@tgross35 tgross35 changed the title Change clippy stubs to be nonpanicking Change f16 and f128 clippy stubs to be nonpanicking Mar 26, 2024
@tgross35
Copy link
Contributor Author

@rustbot label +F-f16_and_f128

@rustbot rustbot added the F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` label Mar 26, 2024
@tgross35 tgross35 force-pushed the clippy-f16-f128-check-stubs branch from 015538f to fcad3ca Compare March 26, 2024 10:01
@tgross35
Copy link
Contributor Author

Thanks @blyxyas, are you able to r+ or waiting for a second look?

blyxyas
blyxyas requested changes Mar 26, 2024
View reviewed changes
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

I was waiting for Fred to review it, but I mean, it's a very small change. I'll just r+ this
(I'll also make a personal note to fix this once f16/f128 is stabilized)

Just a little thing, I wouldn't want to cause a bug because of time-mismatch

src/tools/clippy/clippy_lints/src/float_literal.rs Outdated Show resolved Hide resolved
src/tools/clippy/clippy_lints/src/float_literal.rs Outdated Show resolved Hide resolved
@tgross35
Change f16 and f128 clippy stubs to be nonpanicking
2cfd532 
It turns out there is a bit of a circular dependency - I cannot add
anything to `core` because Clippy fails, and I can't actually add
correct Clippy implementations without new implementations from `core`.

Change some of the Clippy stubs from `unimplemented!` to success values
and leave a FIXME in their place to mitigate this.

Fixes <rust-lang#122587>
@tgross35 tgross35 force-pushed the clippy-f16-f128-check-stubs branch from fcad3ca to 2cfd532 Compare March 26, 2024 17:37
@tgross35
Copy link
Contributor Author

Much appreciated, I made the requested tweaks

@blyxyas
Copy link
Member

blyxyas commented Mar 26, 2024

@bors r+

@bors
Copy link
Contributor

bors commented Mar 26, 2024

📌 Commit 2cfd532 has been approved by blyxyas

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 26, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 26, 2024
Merged
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 26, 2024
@workingjubilee
Rollup merge of rust-lang#123087 - tgross35:clippy-f16-f128-check-stu…
2e010f6 
…bs, r=blyxyas

Change `f16` and `f128` clippy stubs to be nonpanicking

It turns out there is a bit of a circular dependency - I cannot add anything to `core` because Clippy fails, and I can't actually add correct Clippy implementations without new implementations from `core`.

Change some of the Clippy stubs from `unimplemented!` to success values and leave a FIXME in their place to mitigate this.

Fixes <rust-lang#122587>
@workingjubilee workingjubilee mentioned this pull request Mar 26, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 26, 2024
@bors
Auto merge of rust-lang#123108 - matthiaskrgr:rollup-zossklv, r=matth…
a1b4991 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#108675 (Document `adt_const_params` feature in Unstable Book)
 - rust-lang#122120 (Suggest associated type bounds on problematic associated equality bounds)
 - rust-lang#122589 (Fix diagnostics for async block cloning)
 - rust-lang#122835 (Require `DerefMut` and `DerefPure` on `deref!()` patterns when appropriate)
 - rust-lang#123049 (In `ConstructCoroutineInClosureShim`, pass receiver by mut ref, not mut pointer)
 - rust-lang#123055 (enable cargo miri test doctests)
 - rust-lang#123057 (unix fs: Make hurd using explicit new rather than From)
 - rust-lang#123087 (Change `f16` and `f128` clippy stubs to be nonpanicking)
 - rust-lang#123103 (Rename `Inherited` -> `TypeckRootCtxt`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7af95ed into rust-lang:master Mar 27, 2024
11 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 27, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 27, 2024
@rust-timer
Unrolled build for rust-lang#123087
a640997 
Rollup merge of rust-lang#123087 - tgross35:clippy-f16-f128-check-stubs, r=blyxyas

Change `f16` and `f128` clippy stubs to be nonpanicking

It turns out there is a bit of a circular dependency - I cannot add anything to `core` because Clippy fails, and I can't actually add correct Clippy implementations without new implementations from `core`.

Change some of the Clippy stubs from `unimplemented!` to success values and leave a FIXME in their place to mitigate this.

Fixes <rust-lang#122587>
@xFrednet xFrednet assigned blyxyas and unassigned xFrednet Mar 31, 2024
@tgross35 tgross35 deleted the clippy-f16-f128-check-stubs branch July 16, 2024 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blyxyas blyxyas blyxyas requested changes

Assignees

@blyxyas blyxyas

Labels
A-clippy Area: Clippy F-f16_and_f128 `#![feature(f16)]`, `#![feature(f128)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.79.0
Development

Successfully merging this pull request may close these issues.

ICE: not implemented: f16_f128
6 participants
@tgross35 @rustbot @blyxyas @bors @Mark-Simulacrum @xFrednet