Make use of `[wrapping_]byte_{add,sub}` by WaffleLapkin · Pull Request #100819 · rust-lang/rust

WaffleLapkin · 2022-08-20T22:28:30Z

These new methods trivially replace old .cast().wrapping_offset().cast() & similar code.
Note that arith_offset and wrapping_offset are the same thing.

r? @scottmcm

split off from #100746

rustbot · 2022-08-20T22:28:33Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

scottmcm · 2022-08-22T07:44:07Z

library/core/src/slice/iter/macros.rs

This change is fine, but I definitely look at it and wonder if it can be

Suggested change

$self.end = $self.end.wrapping_byte_offset(-$n);

$self.end = $self.end.wrapping_byte_sub($n);

since it's explicitly shrink.

I haven't done this originally because ($n): isize, but I've just checked and it seems that this is only called with literals and smt as isize so this change makes sense.

scottmcm · 2022-08-22T07:46:17Z

@bors r+ rollup

bors · 2022-08-22T07:46:19Z

📌 Commit b2326ae96398eb936351a7364dc9526730f4814a has been approved by scottmcm

It is now in the queue for this repository.

bors · 2022-08-22T21:23:09Z

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

...replacing `.cast().wrapping_offset().cast()` & similar code.

WaffleLapkin · 2022-08-23T15:50:17Z

rebased to make merge conflicts go away

scottmcm · 2022-08-23T18:54:42Z

@bors r+

bors · 2022-08-23T18:54:43Z

📌 Commit 53565b2 has been approved by scottmcm

It is now in the queue for this repository.

… r=scottmcm Make use of `[wrapping_]byte_{add,sub}` These new methods trivially replace old `.cast().wrapping_offset().cast()` & similar code. Note that [`arith_offset`](https://doc.rust-lang.org/std/intrinsics/fn.arith_offset.html) and `wrapping_offset` are the same thing. r? `@scottmcm` _split off from #100746_

… r=scottmcm Make use of `[wrapping_]byte_{add,sub}` These new methods trivially replace old `.cast().wrapping_offset().cast()` & similar code. Note that [`arith_offset`](https://doc.rust-lang.org/std/intrinsics/fn.arith_offset.html) and `wrapping_offset` are the same thing. r? ``@scottmcm`` _split off from #100746_

… r=scottmcm Make use of `[wrapping_]byte_{add,sub}` These new methods trivially replace old `.cast().wrapping_offset().cast()` & similar code. Note that [`arith_offset`](https://doc.rust-lang.org/std/intrinsics/fn.arith_offset.html) and `wrapping_offset` are the same thing. r? ```@scottmcm``` _split off from #100746_

matthiaskrgr · 2022-08-24T16:18:50Z

@bors rollup=iffy #100947 (comment)
(this is just a shot into the dark, feel free to roll it back)

WaffleLapkin · 2022-08-28T11:37:16Z

Can this PR be un-rollup=iffy-ed? Same as #100811 (comment), I don't think it could cause the hang, given that it basically changes nothing.

matthiaskrgr · 2022-08-28T11:52:46Z

@bors rollup=maybe

… r=scottmcm Make use of `[wrapping_]byte_{add,sub}` These new methods trivially replace old `.cast().wrapping_offset().cast()` & similar code. Note that [`arith_offset`](https://doc.rust-lang.org/std/intrinsics/fn.arith_offset.html) and `wrapping_offset` are the same thing. r? `@scottmcm` _split off from #100746_

Rollup of 8 pull requests Successful merges: - rust-lang#98304 (Add MaybeUninit memset test) - rust-lang#98801 (Add a `File::create_new` constructor) - rust-lang#99821 (Remove separate indexing of early-bound regions) - rust-lang#100239 (remove an ineffective check in const_prop) - rust-lang#100337 (Stabilize `std::io::read_to_string`) - rust-lang#100819 (Make use of `[wrapping_]byte_{add,sub}`) - rust-lang#100934 (Remove a panicking branch from `fmt::builders::PadAdapter`) - rust-lang#101000 (Separate CountIsStar from CountIsParam in rustc_parse_format.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

ChrisDenton · 2022-08-31T11:56:26Z

library/alloc/src/vec/into_iter.rs

-            self.end = unsafe { arith_offset(self.end as *const i8, -1) as *mut T };
+            self.end = self.ptr.wrapping_byte_sub(1);


Is this right? It's using self.ptr instead of self.end

Indeed this is wrong; should be fixed by #101237.

I'm sorry 🥲

RalfJung · 2022-08-31T11:58:07Z

I think there's a bug in this PR -- see #101235.

fix into_iter on ZST Fixes rust-lang#101235 Thanks to `@ChrisDenton` for [spotting the problem](rust-lang#100819 (review)).

…leftovers, r=scottmcm Some more cleanup in `core` - remove some integer casts from slice iter (proposed in rust-lang#100819 (comment)) - replace `as usize` casts with `usize::from` in slice sort (proposed in rust-lang#100822 (comment)) r? `@scottmcm`

fix into_iter on ZST Fixes rust-lang/rust#101235 Thanks to `@ChrisDenton` for [spotting the problem](rust-lang/rust#100819 (review)).

…, r=scottmcm Some more cleanup in `core` - remove some integer casts from slice iter (proposed in rust-lang/rust#100819 (comment)) - replace `as usize` casts with `usize::from` in slice sort (proposed in rust-lang/rust#100822 (comment)) r? `@scottmcm`

rust-highfive assigned scottmcm Aug 20, 2022

rustbot added 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 Aug 20, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 20, 2022

scottmcm reviewed Aug 22, 2022

View reviewed changes

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 Aug 22, 2022

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 22, 2022

Make use of [wrapping_]byte_{add,sub}

53565b2

...replacing `.cast().wrapping_offset().cast()` & similar code.

WaffleLapkin force-pushed the use_ptr_byte_methods branch from b2326ae to 53565b2 Compare August 23, 2022 15:33

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 Aug 23, 2022

JohnTitor mentioned this pull request Aug 23, 2022

Rollup of 9 pull requests #100931

Closed

JohnTitor mentioned this pull request Aug 24, 2022

Rollup of 8 pull requests #100937

Closed

matthiaskrgr mentioned this pull request Aug 24, 2022

Rollup of 12 pull requests #100947

Closed

Dylan-DPC mentioned this pull request Aug 29, 2022

Rollup of 7 pull requests #101148

Closed

Dylan-DPC mentioned this pull request Aug 29, 2022

Rollup of 8 pull requests #101152

Merged

bors merged commit 395ce34 into rust-lang:master Aug 29, 2022

rustbot added this to the 1.65.0 milestone Aug 29, 2022

ChrisDenton reviewed Aug 31, 2022

View reviewed changes

RalfJung mentioned this pull request Aug 31, 2022

fix into_iter on ZST #101237

Merged

RalfJung added a commit to RalfJung/rust that referenced this pull request Aug 31, 2022

Rollup merge of rust-lang#101237 - RalfJung:into-iter-zst, r=thomcc

2a02882

fix into_iter on ZST Fixes rust-lang#101235 Thanks to `@ChrisDenton` for [spotting the problem](rust-lang#100819 (review)).

WaffleLapkin deleted the use_ptr_byte_methods branch September 1, 2022 10:36

This was referenced Sep 4, 2022

Some more cleanup in core #101412

Merged

Tracking Issue for pointer_bytes_offsets #96283

Closed

	$self.end = $self.end.wrapping_byte_offset(-$n);
	$self.end = $self.end.wrapping_byte_sub($n);

		self.end = unsafe { arith_offset(self.end as const i8, -1) as mut T };
		self.end = self.ptr.wrapping_byte_sub(1);

Uh oh!

Conversation

WaffleLapkin commented Aug 20, 2022

Uh oh!

rustbot commented Aug 20, 2022

Uh oh!

scottmcm Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin Aug 22, 2022

Choose a reason for hiding this comment

Uh oh!

scottmcm commented Aug 22, 2022

Uh oh!

bors commented Aug 22, 2022

Uh oh!

bors commented Aug 22, 2022

Uh oh!

WaffleLapkin commented Aug 23, 2022

Uh oh!

scottmcm commented Aug 23, 2022

Uh oh!

bors commented Aug 23, 2022

Uh oh!

matthiaskrgr commented Aug 24, 2022

Uh oh!

WaffleLapkin commented Aug 28, 2022

Uh oh!

matthiaskrgr commented Aug 28, 2022

Uh oh!

ChrisDenton Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

RalfJung Aug 31, 2022

Choose a reason for hiding this comment

Uh oh!

WaffleLapkin Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Aug 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants