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 a note saying that {u8,i8}::from_{be,le,ne}_bytes is meaningless #134079

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

Add a note saying that {u8,i8}::from_{be,le,ne}_bytes is meaningless #134079

wants to merge 1 commit into from
+29 −4

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Dec 9, 2024

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Dec 9, 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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 9, 2024
cyrgani
cyrgani reviewed Dec 9, 2024
View reviewed changes
library/core/src/num/mod.rs Outdated Show resolved Hide resolved
@tbu- tbu- force-pushed the pr_doc_x8_to_from_xe_bytes branch from 66b32cf to 7548135 Compare December 9, 2024 19:01
@jhpratt
Copy link
Member

jhpratt commented Dec 10, 2024

I think saying "no-op" would be better than "useless". It does have a purpose, after all (primarily macros).

@scottmcm
Copy link
Member

r? jhpratt

@rustbot rustbot assigned jhpratt and unassigned scottmcm Dec 10, 2024
@jhpratt jhpratt 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 10, 2024
@tbu-
Copy link
Contributor Author

tbu- commented Dec 10, 2024

I think saying "no-op" would be better than "useless". It does have a purpose, after all (primarily macros).

It's not really a no-op, it's taking the byte and putting it into an array. The function name (or documentation or doctest) doesn't really make sense for u8 as a u8 doesn't have a concept of byte order (since it only has a single byte, "byte order" doesn't make sense).

I doubt the existence of them was even thought about when initially introducing them. Looking at the git blame, it was apparently me who added them. :D #51919. I think it's only an accident that they exist on u8/i8 at all.

I'm open for another word that describes that the function's names already convey nonsensical behavior, but I think "no-op" is not it.

@jhpratt
Copy link
Member

jhpratt commented Dec 10, 2024

It's not really a no-op, it's taking the byte and putting it into an array.

True, I spoke too quickly. In any situation, "useless" probably isn't the right word here. I don't have any concrete alternatives off-hand.

@tbu- tbu- force-pushed the pr_doc_x8_to_from_xe_bytes branch from 7548135 to 1f64f99 Compare December 10, 2024 22:53
@tbu- tbu- changed the title Add a note saying that {u8,i8}::from_{be,le,ne}_bytes is useless Add a note saying that {u8,i8}::from_{be,le,ne}_bytes is meaningless Dec 10, 2024
@tbu-
Copy link
Contributor Author

tbu- commented Dec 10, 2024

Changed to "meaningless".

@jhpratt
Copy link
Member

jhpratt commented Dec 11, 2024

Might want to double check, as it doesn't look like that actually happened 🙂

@jhpratt
Copy link
Member

jhpratt commented Dec 11, 2024

@bors delegate+

@bors
Copy link
Contributor

bors commented Dec 11, 2024

✌️ @tbu-, you can now approve this pull request!

If @jhpratt told you to "r=me" after making some further change, please make that change, then do @bors r=@jhpratt

@tbu- tbu- force-pushed the pr_doc_x8_to_from_xe_bytes branch from 1f64f99 to e37d7c0 Compare December 11, 2024 01:18
@tbu-
Copy link
Contributor Author

tbu- commented Dec 11, 2024

You're right, I forgot to git add the changes.

@bors r=@jhpratt

@bors
Copy link
Contributor

bors commented Dec 11, 2024

📌 Commit e37d7c0 has been approved by jhpratt

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 Dec 11, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 11, 2024
@jhpratt
Rollup merge of rust-lang#134079 - tbu-:pr_doc_x8_to_from_xe_bytes, r…
43b4af5 
…=jhpratt

Add a note saying that `{u8,i8}::from_{be,le,ne}_bytes` is meaningless
@jhpratt jhpratt mentioned this pull request Dec 11, 2024
Open
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2024
@bors
Auto merge of rust-lang#134164 - jhpratt:rollup-s7z0vcc, r=jhpratt
0c8422c 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#134079 (Add a note saying that `{u8,i8}::from_{be,le,ne}_bytes` is meaningless)
 - rust-lang#134105 (Validate self in host predicates correctly)
 - rust-lang#134136 (Exercise const trait interaction with default fields)
 - rust-lang#134139 ([AIX] keep profile-rt symbol alive)
 - rust-lang#134141 (Remove more traces of anonymous ADTs)
 - rust-lang#134142 (Rudimentary heuristic to insert parentheses when needed for RPIT overcaptures lint)
 - rust-lang#134158 (Rename `projection_def_id` to `item_def_id`)
 - rust-lang#134160 (Add vacation entry for myself in triagebot.toml)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyrgani cyrgani cyrgani left review comments

Assignees

@jhpratt jhpratt

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tbu- @rustbot @jhpratt @scottmcm @bors @cyrgani