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

Remove hir::ArrayLen #133589

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Remove hir::ArrayLen #133589

merged 1 commit into from
Dec 2, 2024

Conversation

voidc
Copy link
Contributor

@voidc voidc commented Nov 28, 2024

This refactoring removes hir::ArrayLen, replacing it with hir::ConstArg. To represent inferred array lengths (previously hir::ArrayLen::Infer), a new variant ConstArgKind::Infer is added.

r? @BoxyUwU

@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-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 28, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@voidc
Copy link
Contributor Author

voidc commented Nov 29, 2024

- | 1: user_ty: Canonical { value: TypeOf(DefId(0:3 ~ issue_99325[d56d]::function_with_bytes), UserArgs { args: [UnevaluatedConst { def: DefId(0:8 ~ issue_99325[d56d]::main::{constant#1}), args: [] }], user_self_ty: None }), max_universe: U0, variables: [] }, span: $DIR/issue_99325.rs:14:16: 14:68, inferred_ty: fn() -> &'static [u8] {function_with_bytes::<&*b"AAAA">}
+ | 1: user_ty: Canonical { value: TypeOf(DefId(0:3 ~ issue_99325[d56d]::function_with_bytes), UserArgs { args: [&*b"AAAA"], user_self_ty: None }), max_universe: U0, variables: [] }, span: $DIR/issue_99325.rs:14:16: 14:68, inferred_ty: fn() -> &'static [u8] {function_with_bytes::<&*b"AAAA">}

I think this is because I now normalize all ConstArgs in lower_const_arg. The diff doesn't look wrong to me, can I just bless it?

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool :3 thanks for doing this

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/librustdoc/clean/types.rs Show resolved Hide resolved
@@ -2,7 +2,7 @@

| User Type Annotations
| 0: user_ty: Canonical { value: TypeOf(DefId(0:3 ~ issue_99325[d56d]::function_with_bytes), UserArgs { args: [&*b"AAAA"], user_self_ty: None }), max_universe: U0, variables: [] }, span: $DIR/issue_99325.rs:13:16: 13:46, inferred_ty: fn() -> &'static [u8] {function_with_bytes::<&*b"AAAA">}
| 1: user_ty: Canonical { value: TypeOf(DefId(0:3 ~ issue_99325[d56d]::function_with_bytes), UserArgs { args: [UnevaluatedConst { def: DefId(0:8 ~ issue_99325[d56d]::main::{constant#1}), args: [] }], user_self_ty: None }), max_universe: U0, variables: [] }, span: $DIR/issue_99325.rs:14:16: 14:68, inferred_ty: fn() -> &'static [u8] {function_with_bytes::<&*b"AAAA">}
| 1: user_ty: Canonical { value: TypeOf(DefId(0:3 ~ issue_99325[d56d]::function_with_bytes), UserArgs { args: [&*b"AAAA"], user_self_ty: None }), max_universe: U0, variables: [] }, span: $DIR/issue_99325.rs:14:16: 14:68, inferred_ty: fn() -> &'static [u8] {function_with_bytes::<&*b"AAAA">}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change would be why we dont want to normalize at the end of lower_const_arg. User type annotations exist to remember the "unnormalized as written by the user" types/generic arguments in MIR so that any lifetime constraints introduced by them can be known to the borrow checker instead of being ignored :3

This specific example isn't wrong per se but in the general case it would be with const arguments that use lifetime parameters, e.g.:

#![feature(min_generic_const_args)]

trait Trait<'a> {
  const ASSOC: usize;
}

impl Trait<'static> for u8 {
  const ASSOC: usize = 10;
}

struct Bar<const N: usize>;

fn foo<'a>() {
  let _: Bar<<u8 as Trait<'a>>::ASSOC>;
}

<u8 as Trait<'a>>::ASSOC being well formed requires 'a: 'static which doesn't hold so we should error in borrow checking. This is accomplished by having a user type annotation of <u8 as Trait<'a>>::ASSOC so that borrow checking knows 'a: 'static must hold. With this change we'd have a user type annotation of 10 which doesn't tell borrow checking anything about 'a: 'static having to hold :3

Though this example won't actually do anything right now since min_generic_const_args hasn't been implemented :3

compiler/rustc_hir/src/hir.rs Outdated Show resolved Hide resolved
@voidc voidc marked this pull request as ready for review November 30, 2024 10:36
@rustbot
Copy link
Collaborator

rustbot commented Nov 30, 2024

HIR ty lowering was modified

cc @fmease

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you squash these commits then r=me

@aDotInTheVoid
Copy link
Member

@bors r=boxyuwu

@bors
Copy link
Contributor

bors commented Dec 1, 2024

📌 Commit d38f013 has been approved by boxyuwu

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 1, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2024
Rollup of 4 pull requests

Successful merges:

 - rust-lang#133589 (Remove `hir::ArrayLen`)
 - rust-lang#133672 (Remove a bunch of unnecessary const stability noise)
 - rust-lang#133678 (Stabilize `ptr::fn_addr_eq`)
 - rust-lang#133727 (Update mailmap)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2024
Rollup of 4 pull requests

Successful merges:

 - rust-lang#133589 (Remove `hir::ArrayLen`)
 - rust-lang#133672 (Remove a bunch of unnecessary const stability noise)
 - rust-lang#133678 (Stabilize `ptr::fn_addr_eq`)
 - rust-lang#133727 (Update mailmap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 811eaeb into rust-lang:master Dec 2, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 2, 2024
Rollup merge of rust-lang#133589 - voidc:remove-array-len, r=boxyuwu

Remove `hir::ArrayLen`

This refactoring removes `hir::ArrayLen`, replacing it with `hir::ConstArg`. To represent inferred array lengths (previously `hir::ArrayLen::Infer`), a new variant `ConstArgKind::Infer` is added.

r? `@BoxyUwU`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024
…id, r=compiler-errors

Use correct `hir_id` for array const arg infers

Fixes rust-lang#133771

`self.next_id()` results in the `DefId` for the const argument, created from the hack introduced by rust-lang#133468, having no `HirId` associated with it. This then results in an ICE in metadata encoding. Fixing this then results in *another* ICE where `encode_defs` was not skipping encoding `type_of` and other queries for `DefId`s when they correspond to a `ConstArgKind::Infer` node.

This only reproduces with a library crate as metadata is not encoded for binaries, and apparently we had 0 tests for `generic_arg_infer` for array lengths in a library crate so this was not caught :<

cc rust-lang#133589 `@voidc`

r? `@compiler-errors` `@lcnr`
jhpratt added a commit to jhpratt/rust that referenced this pull request Dec 3, 2024
…id, r=compiler-errors

Use correct `hir_id` for array const arg infers

Fixes rust-lang#133771

`self.next_id()` results in the `DefId` for the const argument, created from the hack introduced by rust-lang#133468, having no `HirId` associated with it. This then results in an ICE in metadata encoding. Fixing this then results in *another* ICE where `encode_defs` was not skipping encoding `type_of` and other queries for `DefId`s when they correspond to a `ConstArgKind::Infer` node.

This only reproduces with a library crate as metadata is not encoded for binaries, and apparently we had 0 tests for `generic_arg_infer` for array lengths in a library crate so this was not caught :<

cc rust-lang#133589 ``@voidc``

r? ``@compiler-errors`` ``@lcnr``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024
…id, r=compiler-errors

Use correct `hir_id` for array const arg infers

Fixes rust-lang#133771

`self.next_id()` results in the `DefId` for the const argument, created from the hack introduced by rust-lang#133468, having no `HirId` associated with it. This then results in an ICE in metadata encoding. Fixing this then results in *another* ICE where `encode_defs` was not skipping encoding `type_of` and other queries for `DefId`s when they correspond to a `ConstArgKind::Infer` node.

This only reproduces with a library crate as metadata is not encoded for binaries, and apparently we had 0 tests for `generic_arg_infer` for array lengths in a library crate so this was not caught :<

cc rust-lang#133589 ```@voidc```

r? ```@compiler-errors``` ```@lcnr```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 3, 2024
…id, r=compiler-errors

Use correct `hir_id` for array const arg infers

Fixes rust-lang#133771

`self.next_id()` results in the `DefId` for the const argument, created from the hack introduced by rust-lang#133468, having no `HirId` associated with it. This then results in an ICE in metadata encoding. Fixing this then results in *another* ICE where `encode_defs` was not skipping encoding `type_of` and other queries for `DefId`s when they correspond to a `ConstArgKind::Infer` node.

This only reproduces with a library crate as metadata is not encoded for binaries, and apparently we had 0 tests for `generic_arg_infer` for array lengths in a library crate so this was not caught :<

cc rust-lang#133589 `@voidc`

r? `@compiler-errors` `@lcnr`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 3, 2024
Rollup merge of rust-lang#133779 - BoxyUwU:array_const_arg_infer_hir_id, r=compiler-errors

Use correct `hir_id` for array const arg infers

Fixes rust-lang#133771

`self.next_id()` results in the `DefId` for the const argument, created from the hack introduced by rust-lang#133468, having no `HirId` associated with it. This then results in an ICE in metadata encoding. Fixing this then results in *another* ICE where `encode_defs` was not skipping encoding `type_of` and other queries for `DefId`s when they correspond to a `ConstArgKind::Infer` node.

This only reproduces with a library crate as metadata is not encoded for binaries, and apparently we had 0 tests for `generic_arg_infer` for array lengths in a library crate so this was not caught :<

cc rust-lang#133589 `@voidc`

r? `@compiler-errors` `@lcnr`
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 15, 2024
Remove `hir::ArrayLen`

This refactoring removes `hir::ArrayLen`, replacing it with `hir::ConstArg`. To represent inferred array lengths (previously `hir::ArrayLen::Infer`), a new variant `ConstArgKind::Infer` is added.

r? `@BoxyUwU`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants