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.

Sign up for GitHub

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

Jump to bottom

Allow struct field default values to reference struct's generics #137913

Merged
merged 1 commit into from
Mar 5, 2025
Merged

Allow struct field default values to reference struct's generics #137913

merged 1 commit into from
Mar 5, 2025
+112 −19

Conversation

compiler-errors
Copy link
Member

Right now, the default field value feature (#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the generics_of parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? @BoxyUwU or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes #137896

@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. labels Mar 3, 2025
@compiler-errors compiler-errors force-pushed the struct-field-default-generic branch from 4a040a0 to fdab679 Compare March 3, 2025 05:58
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 3, 2025

Can we have a test of making use of a defaulted field that is too generic to be evaluated. e.g. Foo::<T> { .. } where field: Vec<T> = Vec::new(),. Same w/ an example where when a concrete argument for T is monomorphized we get a post mono error?

I do think it makes sense to support generics here though, it was specified in the RFC they should be permitted 🤷‍♀️ These anon consts don't flow into the type system so I see no issues from a types/const generics POV.

@BoxyUwU BoxyUwU 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2025
@compiler-errors compiler-errors force-pushed the struct-field-default-generic branch from fdab679 to 95a30bb Compare March 3, 2025 23:49
@compiler-errors
Copy link
Member Author

Can we have a test of making use of a defaulted field that is too generic to be evaluated.

I just used X + 1 :clueless: lmk if you have a more interesting expr.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 3, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 4, 2025

Oh I meant cases where the struct expr doesn't provide concrete arguments to the generic parameters being depended on, so that the anon const is still too generic to be evaluated during typeck.

@compiler-errors compiler-errors force-pushed the struct-field-default-generic branch from 95a30bb to 3e5fddc Compare March 4, 2025 01:01
@BoxyUwU
Copy link
Member

BoxyUwU commented Mar 4, 2025

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 4, 2025

📌 Commit 3e5fddc 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 Mar 4, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2025
@matthiaskrgr
Rollup merge of rust-lang#137913 - compiler-errors:struct-field-defau…
81839ed 
…lt-generic, r=BoxyUwU

Allow struct field default values to reference struct's generics

Right now, the default field value feature (rust-lang#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the `generics_of` parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? `@boxyuwu` or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes rust-lang#137896
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 4, 2025
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2025
@matthiaskrgr
Rollup merge of rust-lang#137913 - compiler-errors:struct-field-defau…
3def7c7 
…lt-generic, r=BoxyUwU

Allow struct field default values to reference struct's generics

Right now, the default field value feature (rust-lang#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the `generics_of` parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? ``@boxyuwu`` or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes rust-lang#137896
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 4, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2025
@bors
Auto merge of rust-lang#137982 - matthiaskrgr:rollup-12jvilg, r=matth…
99cc250 
…iaskrgr

Rollup of 12 pull requests

Successful merges:

 - rust-lang#136975 (Look for `python3` first on MacOS, not `py`)
 - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137463 ([illumos] attempt to use posix_spawn to spawn processes)
 - rust-lang#137722 (`librustdoc`: 2024 edition! 🎊)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)

r? `@ghost`
`@rustbot` modify labels: rollup
tgross35 added a commit to tgross35/rust that referenced this pull request Mar 4, 2025
@tgross35
Rollup merge of rust-lang#137913 - compiler-errors:struct-field-defau…
ce0f32c 
…lt-generic, r=BoxyUwU

Allow struct field default values to reference struct's generics

Right now, the default field value feature (rust-lang#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the `generics_of` parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? ```@boxyuwu``` or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes rust-lang#137896
@tgross35 tgross35 mentioned this pull request Mar 4, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2025
@bors
Auto merge of rust-lang#137989 - tgross35:rollup-30jvjyh, r=tgross35
8fa6a41 
Rollup of 6 pull requests

Successful merges:

 - rust-lang#135695 (Support raw-dylib link kind on ELF)
 - rust-lang#137836 (Set `target_vendor = "openwrt"` on `mips64-openwrt-linux-musl`)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)

r? `@ghost`
`@rustbot` modify labels: rollup
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 4, 2025
@workingjubilee
Rollup merge of rust-lang#137913 - compiler-errors:struct-field-defau…
58dd700 
…lt-generic, r=BoxyUwU

Allow struct field default values to reference struct's generics

Right now, the default field value feature (rust-lang#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the `generics_of` parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? ````@boxyuwu```` or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes rust-lang#137896
@workingjubilee workingjubilee mentioned this pull request Mar 4, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
@bors
Auto merge of rust-lang#138020 - workingjubilee:rollup-eva637e, r=wor…
37d6b94 
…kingjubilee

Rollup of 33 pull requests

Successful merges:

 - rust-lang#136581 (Retire the legacy `Makefile`-based `run-make` test infra)
 - rust-lang#136764 (Make `ptr_cast_add_auto_to_object` lint into hard error)
 - rust-lang#136798 (Added documentation for flushing per rust-lang#74348)
 - rust-lang#136865 (Perform deeper compiletest path normalization for `$TEST_BUILD_DIR` to account for compare-mode/debugger cases, and normalize long type file filename hashes)
 - rust-lang#136975 (Look for `python3` first on MacOS, not `py`)
 - rust-lang#137240 (Slightly reformat `std::fs::remove_dir_all` error docs)
 - rust-lang#137303 (Remove `MaybeForgetReturn` suggestion)
 - rust-lang#137327 (Undeprecate env::home_dir)
 - rust-lang#137502 (Don't include global asm in `mir_keys`, fix error body synthesis)
 - rust-lang#137534 ([rustdoc] hide item that is not marked as doc(inline) and whose src is doc(hidden))
 - rust-lang#137565 (Try to point of macro expansion from resolver and method errors if it involves macro var)
 - rust-lang#137643 (Add DWARF test case for non-C-like `repr128` enums)
 - rust-lang#137758 (fix usage of ty decl macro fragments in attributes)
 - rust-lang#137764 (Ensure that negative auto impls are always applicable)
 - rust-lang#137772 (Fix char count in `Display` for `ByteStr`)
 - rust-lang#137798 (ci: use ubuntu 24 on arm large runner)
 - rust-lang#137805 (adjust Layout debug printing to match the internal field name)
 - rust-lang#137808 (Do not require that unsafe fields lack drop glue)
 - rust-lang#137820 (Clarify why InhabitedPredicate::instantiate_opt exists)
 - rust-lang#137825 (Provide more context on resolve error caused from incorrect RTN)
 - rust-lang#137829 (Stabilize [T]::split_off... methods)
 - rust-lang#137834 (rustc_fluent_macro: use CARGO_CRATE_NAME instead of CARGO_PKG_NAME)
 - rust-lang#137850 (Stabilize `box_uninit_write`)
 - rust-lang#137912 (Do not recover missing lifetime with random in-scope lifetime)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137923 (Simplify `<Postorder as Iterator>::size_hint`)
 - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)
 - rust-lang#137986 (Fix some typos)
 - rust-lang#137991 (Add `avr-none` to SUMMARY.md and platform-support.md)
 - rust-lang#137993 (Remove obsolete comment from DeduceReadOnly)
 - rust-lang#137996 (Revert "compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync"")

r? `@ghost`
`@rustbot` modify labels: rollup
@workingjubilee workingjubilee mentioned this pull request Mar 5, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
@bors
Auto merge of rust-lang#138031 - workingjubilee:rollup-5bsotpz, r=wor…
4559163 
…kingjubilee

Rollup of 15 pull requests

Successful merges:

 - rust-lang#137829 (Stabilize [T]::split_off... methods)
 - rust-lang#137850 (Stabilize `box_uninit_write`)
 - rust-lang#137912 (Do not recover missing lifetime with random in-scope lifetime)
 - rust-lang#137913 (Allow struct field default values to reference struct's generics)
 - rust-lang#137923 (Simplify `<Postorder as Iterator>::size_hint`)
 - rust-lang#137949 (Update MSVC INSTALL.md instructions to recommend VS 2022 + recent Windows 10/11 SDK)
 - rust-lang#137963 (Add ``dyn`` keyword to `E0373` examples)
 - rust-lang#137975 (Remove unused `PpMode::needs_hir`)
 - rust-lang#137981 (rustdoc search: increase strictness of typechecking)
 - rust-lang#137986 (Fix some typos)
 - rust-lang#137991 (Add `avr-none` to SUMMARY.md and platform-support.md)
 - rust-lang#137993 (Remove obsolete comment from DeduceReadOnly)
 - rust-lang#137996 (Revert "compiler/rustc_data_structures/src/sync/worker_local.rs: delete "unsafe impl Sync"")
 - rust-lang#138019 (Pretty-print `#[deprecated]` attribute in HIR.)
 - rust-lang#138026 (Make CrateItem::body() function return an option)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b3d7c14 into rust-lang:master Mar 5, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 5, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
@rust-timer
Unrolled build for rust-lang#137913
34ae93e 
Rollup merge of rust-lang#137913 - compiler-errors:struct-field-default-generic, r=BoxyUwU

Allow struct field default values to reference struct's generics

Right now, the default field value feature (rust-lang#132162) lowers anon consts whose types may reference ADT params that the const doesn't inherit.

This PR fixes this, so that these defaults can reference ADTs' generics, and sets the `generics_of` parenting up correctly.

There doesn't seem to be a good reason not to support this, since the anon const has a well-defined type from the field, and the anon const doesn't interact with the type system like generic parameter defaults do.

r? `````@boxyuwu````` or reassign

I could also make this into an error if this seems problematic (https://github.com/rust-lang/rust/compare/master...compiler-errors:rust:default-field-value-implicit-param?expand=1)...... but I'd rather make this work and register an open question on the tracking issue about validating that this is well-vetted.

Fixes rust-lang#137896
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@BoxyUwU BoxyUwU

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.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.

ICE: sty: called Option::unwrap() on a None value
4 participants
@compiler-errors @BoxyUwU @bors @rustbot