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.

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

Implement Deref{Mut} for arrays #108650

Closed
wants to merge 3 commits into from
Closed

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Mar 2, 2023

Now that we have const generics, there shouldn't really be a reason for arrays not to implement Deref/DerefMut anymore (to my knowledge). So this PR is an attempt in making that work.

With this, arrays now participate in deref coercions.

Unfortunately we still need to special case arrays in method probing, as unsizing can be always done in a const context while deref cannot on stable and currently the compiler will prefer dereferencing over unsizing. Of note is that once Deref is stably const, the unsizing step can (probably, not tested) be fully removed, meaning Unsize adjustments no longer need to happen for arrays)

Fixes #62992

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2023

r? @Nilstrieb

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Mar 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2023

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

@Noratrieb
Copy link
Member

Noratrieb commented Mar 2, 2023

@compiler-errors on the compiler part, for the rest
r? libs-api

@rustbot rustbot assigned Amanieu and unassigned Noratrieb Mar 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2023

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@Veykril
Copy link
Member Author

Veykril commented Mar 2, 2023

Not sure if this might be a T-lang thing as well, since arrays now participate in deref coercions (and arrays being a language type opposed to library)

@compiler-errors
Copy link
Member

compiler-errors commented Mar 2, 2023

I actually tried to implement this back in #92652. I don't think it makes sense to implement Deref in this case anymore, since array -> slice is an unsize coercion.

Regardless, needs an ACP from T-libs-api I think? https://std-dev-guide.rust-lang.org/feature-lifecycle/api-change-proposals.html

@Veykril
Copy link
Member Author

Veykril commented Mar 2, 2023

Guess I wasn't looking thoroughly enough for a previous PR :) I can see the reasoning behind why not to add this from eddy's comment.

Turns out this is even the third PR trying to do this, I wonder if it makes sense to add a comment somewhere that states as to why we don't want this impl?

@Veykril Veykril closed this Mar 2, 2023
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
Prepare all required actions
Getting action download info
Download action repository 'actions/checkout@v3' (SHA:ac593985615ec2ede58e132d2e21d2b1cbd6127c)
Download action repository 'rust-lang/simpleinfra@master' (SHA:0fdabd83e1d3faaa8e9cfd7c00031e3a92997344)
Complete job name: PR (x86_64-gnu-llvm-14, false, ubuntu-20.04-xl)
git config --global core.autocrlf false
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
env:
  CI_JOB_NAME: x86_64-gnu-llvm-14
---
Successfully built 1a4e86b77eb4
Successfully tagged rust-ci:latest
Built container sha256:1a4e86b77eb4d273114a7fec4cee1862970c8aef0cd22ab0d3cd26294422753f
Uploading finished image to https://ci-caches.rust-lang.org/docker/2d8760e15cd716e46ea98fab8e7af60c712922f5df298e54391d8a5a58bcaa16a7d6f2f012a2cb04e2fd5f51945d9112e87fb2970f7c4b52f59f1373bcbb7693
upload failed: - to s3://rust-lang-ci-sccache2/docker/2d8760e15cd716e46ea98fab8e7af60c712922f5df298e54391d8a5a58bcaa16a7d6f2f012a2cb04e2fd5f51945d9112e87fb2970f7c4b52f59f1373bcbb7693 Unable to locate credentials
[CI_JOB_NAME=x86_64-gnu-llvm-14]
---
failures:

---- [ui] tests/ui/coercion/issue-73886.rs stdout ----

error: /checkout/tests/ui/coercion/issue-73886.rs:2: expected error not found: non-primitive cast: `&&[i32; 1]` as `&[_]`
error: 0 unexpected errors found, 1 expected errors not found
status: exit status: 1
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/coercion/issue-73886.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--remap-path-prefix=/checkout/tests/ui=fake-test-src-base" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/coercion/issue-73886" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/coercion/issue-73886/auxiliary"
    Error {
        line_num: 2,
        kind: Some(
            Error,
            Error,
        ),
        msg: "non-primitive cast: `&&[i32; 1]` as `&[_]`",
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
]

thread '[ui] tests/ui/coercion/issue-73886.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:1421:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

slice methods could be added to the array docs
6 participants