Add separate array length function

[shamatar]

Solves #86403

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @scottmcm (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[jonas-schievink]

This is a breaking change, because the method is unstable

[shamatar]

If it's accepted as insta-stable then it is actually not breaking. I can make it this way for CI purposes

[scottmcm]

Hmm, is this one of the places where the "avoid unstable methods in name resolution" fix kicks in?

Maybe add a ui test, @shamatar, to show what people not using the feature will get, to find out?

[scottmcm]

Ah, those CI results are enough to say that this is definitely breaking.

I'd suggest making a thread in #t-libs on zulip to ask libs how they feel about doing this.

[shamatar]

I'm very surprised that unstable method resolution didn't work with it.

So far I'd still argue to mark it as stable and make a crater run with it, but let me ask on Zullip

[shamatar]

Duplicating from Zullip

I've made a simple MIR optimization test for a code like

// EMIT_MIR lower_slice_len.array_bound.SimplifyLocals.diff
pub fn array_bound<const N: usize>(index: usize, slice: &[u8; N]) -> u8 {
    if index < slice.len() {
        slice[index]
    } else {
        42
    }
}

(hacking into SimplifyLocals as the one of the last passes for simplicity, so after any constant prop, and mir-opt-level = 3)

here is a MIR

- // MIR for `array_bound` before SimplifyLocals
+ // MIR for `array_bound` after SimplifyLocals
  
  fn array_bound(_1: usize, _2: &[u8; N]) -> u8 {
      debug index => _1;                   // in scope 0 at $DIR/lower_slice_len.rs:14:36: 14:41
      debug slice => _2;                   // in scope 0 at $DIR/lower_slice_len.rs:14:50: 14:55
      let mut _0: u8;                      // return place in scope 0 at $DIR/lower_slice_len.rs:14:70: 14:72
      let mut _3: bool;                    // in scope 0 at $DIR/lower_slice_len.rs:15:8: 15:27
      let mut _4: usize;                   // in scope 0 at $DIR/lower_slice_len.rs:15:8: 15:13
      let mut _5: usize;                   // in scope 0 at $DIR/lower_slice_len.rs:15:16: 15:27
      let mut _6: &[u8];                   // in scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
      let mut _7: &[u8; N];                // in scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
      let _8: usize;                       // in scope 0 at $DIR/lower_slice_len.rs:16:15: 16:20
      let mut _9: usize;                   // in scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
      let mut _10: bool;                   // in scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
  
      bb0: {
          StorageLive(_3);                 // scope 0 at $DIR/lower_slice_len.rs:15:8: 15:27
          StorageLive(_4);                 // scope 0 at $DIR/lower_slice_len.rs:15:8: 15:13
          _4 = _1;                         // scope 0 at $DIR/lower_slice_len.rs:15:8: 15:13
          StorageLive(_5);                 // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:27
          StorageLive(_6);                 // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
          StorageLive(_7);                 // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
          _7 = _2;                         // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
          _6 = move _7 as &[u8] (Pointer(Unsize)); // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:21
          StorageDead(_7);                 // scope 0 at $DIR/lower_slice_len.rs:15:20: 15:21
          _5 = Len((*_6));                 // scope 0 at $DIR/lower_slice_len.rs:15:16: 15:27
          StorageDead(_6);                 // scope 0 at $DIR/lower_slice_len.rs:15:26: 15:27
          _3 = Lt(move _4, move _5);       // scope 0 at $DIR/lower_slice_len.rs:15:8: 15:27
          StorageDead(_5);                 // scope 0 at $DIR/lower_slice_len.rs:15:26: 15:27
          StorageDead(_4);                 // scope 0 at $DIR/lower_slice_len.rs:15:26: 15:27
          switchInt(move _3) -> [false: bb2, otherwise: bb1]; // scope 0 at $DIR/lower_slice_len.rs:15:5: 19:6
      }
  
      bb1: {
          StorageLive(_8);                 // scope 0 at $DIR/lower_slice_len.rs:16:15: 16:20
          _8 = _1;                         // scope 0 at $DIR/lower_slice_len.rs:16:15: 16:20
          _9 = const N;                    // scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
          _10 = Lt(_8, _9);                // scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
          assert(move _10, "index out of bounds: the length is {} but the index is {}", move _9, _8) -> bb3; // scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
      }
  
      bb2: {
          _0 = const 42_u8;                // scope 0 at $DIR/lower_slice_len.rs:18:9: 18:11
          goto -> bb4;                     // scope 0 at $DIR/lower_slice_len.rs:15:5: 19:6
      }
  
      bb3: {
          _0 = (*_2)[_8];                  // scope 0 at $DIR/lower_slice_len.rs:16:9: 16:21
          StorageDead(_8);                 // scope 0 at $DIR/lower_slice_len.rs:17:5: 17:6
          goto -> bb4;                     // scope 0 at $DIR/lower_slice_len.rs:15:5: 19:6
      }
  
      bb4: {
          StorageDead(_3);                 // scope 0 at $DIR/lower_slice_len.rs:19:5: 19:6
          return;                          // scope 0 at $DIR/lower_slice_len.rs:20:2: 20:2
      }
  }

If I make a library modification then _5 = Len((*_6)); becomes _5 = N;

At the moment it's not even Len strictly speaking, but #86383 makes it so, before it was even core::slice::<impl [u8]>::len call instead of Len

[shamatar]

r=@scottmcm It passes all the tests on local, can you please check my guess on making it insta-stable and may be start a CI and potentially crater run?

rust/library/core/src/array/mod.rs

Lines 400 to 405 in 189342d

	#[cfg(not(bootstrap))]
	#[doc(alias = "length")]
	#[stable(feature = "rust1", since = "1.0.0")]
	#[rustc_const_stable(feature = "array_len", since = "1.53.0")]
	#[inline]
	pub const fn len(&self) -> usize {

[shamatar]

"Waiting for review" is not assigned even after r=@, is there any other way to touch it?

[bjorn3]

r=@... is not an existing command. Note that comments of the kind r=me are just informal messages that it is fine for someone else with review permission to do @\bors r=<name of that person> once the review comments are deemed to be resolved by the one using bors.

Everyone can use the following though:

@rustbot label -S-waiting-on-author +S-waiting-on-review

[scottmcm]

Since this is now insta-stable, I've marked it as needs-fcp and will send it over to

r? @m-ou-se

as they're active in the zulip conversation about it.

[shamatar]

Workaround of this would be #86525

[bjorn3]

@bors try @rust-timer queue

per the request in #86525 (comment)

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 189342d with merge 2e412375b3fd677ce07a0ddbd9809b06316d15d1...

[bors]

☀️ Try build successful - checks-actions
Build commit: 2e412375b3fd677ce07a0ddbd9809b06316d15d1 (2e412375b3fd677ce07a0ddbd9809b06316d15d1)

[rust-timer]

Queued 2e412375b3fd677ce07a0ddbd9809b06316d15d1 with parent 8cb207a, future comparison URL.

[rust-timer]

Finished benchmarking try commit (2e412375b3fd677ce07a0ddbd9809b06316d15d1): comparison url.

Summary: This benchmark run did not return any significant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[bjorn3]

On average this seems to be a slight perf improvement up to 0.6%, but there are also several regressions up to 0.8%.

[shamatar]

That performance run was to compare against MIR pass that achieves the same, but I'd expect it to be neutral at the end of the day, and it looks so

[camelid]

I think the #[stable] feature and version should be the same as the one for #[rustc_const_stable]?

[camelid]

Workaround of this would be #86525

That PR has merged now, so I think this one can be closed, right?