-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Rename overflowing_{add,sub,mul} intrinsics to wrapping_{add,sub,mul}. #63642
Conversation
Bootstrap changes look fine. |
@@ -67,7 +67,7 @@ pub fn intrisic_operation_unsafety(intrinsic: &str) -> hir::Unsafety { | |||
match intrinsic { | |||
"size_of" | "min_align_of" | "needs_drop" | | |||
"add_with_overflow" | "sub_with_overflow" | "mul_with_overflow" | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these be changed to match the others, or is their different signature sufficient to motivate a different naming convention? (this naming convention is not reflected in the std names)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These match the LLVM intrinsics. We could rename them later but they're not outright wrong like overflowing_*
intrinsics were.
r=me on the code changes, don't have an opinion on the rustbuild stuff though. |
@bors r=rkruppe,Mark-Simulacrum |
📌 Commit 892ef6f has been approved by |
…mulacrum Rename overflowing_{add,sub,mul} intrinsics to wrapping_{add,sub,mul}. These confused @Gankra, and then, also me, especially since `overflowing_*` *methods* also exist, but they map to `*_with_overflow` intrinsics! r? @oli-obk / @nikomatsakis cc @Mark-Simulacrum (on the rustbuild workaround)
…mulacrum Rename overflowing_{add,sub,mul} intrinsics to wrapping_{add,sub,mul}. These confused @Gankra, and then, also me, especially since `overflowing_*` *methods* also exist, but they map to `*_with_overflow` intrinsics! r? @oli-obk / @nikomatsakis cc @Mark-Simulacrum (on the rustbuild workaround)
Rollup of 6 pull requests Successful merges: - #63149 (resolve: Populate external modules in more automatic and lazy way) - #63545 (Feature gate 'yield $expr?' pre-expansion) - #63548 (Update rustc-demangle to 0.1.16.) - #63558 (Remap paths for proc-macro crates.) - #63641 (add git keyword to submodule comments in config.example.toml) - #63642 (Rename overflowing_{add,sub,mul} intrinsics to wrapping_{add,sub,mul}.) Failed merges: r? @ghost
cmd.arg("--cfg").arg("bootstrap"); | ||
} else { | ||
// NOTE(eddyb) see FIXME above, except now we need annotations again in core. | ||
cmd.arg("--cfg").arg("boostrap_stdarch_ignore_this"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boo strap? 👻
These confused @Gankra, and then, also me, especially since
overflowing_*
methods also exist, but they map to*_with_overflow
intrinsics!r? @oli-obk / @nikomatsakis cc @Mark-Simulacrum (on the rustbuild workaround)