Repository navigation
Conversation
rustbot
added
S-waiting-on-author
fmease
changed the title
b9af36a to
a624fa8
Compare
| let packed = adt_def.repr().packed(); | ||
|
|
||
| let own_params_require_monomorphization = | ||
| LazyCell::new(|| tcx.generics_of(item).own_requires_monomorphization()); |
There was a problem hiding this comment.
LazyCell probably overkill/unnecessary -- gonna perf this at some point
| pub bat: u8 = 130 + 130, | ||
| //~^ ERROR attempt to compute `130_u8 + 130_u8`, which would overflow |
There was a problem hiding this comment.
I would still want us to stabilize this feature with some way this check still happens... Either way, we can probably land this change by adding a FIXME in the test and the code?
There was a problem hiding this comment.
I'd rather not ship this PR with a new stabilization-blocking FIXME since that wouldn't buy us anything, as we'd just kick the can down the road. I'd prefer to settle this either in this PR or in an alternative PR that would get shipped instead. After all, this topic is the only known major blocker, right?
I'm open to alternative proposals from you, T-types or anyone else (as long as they're principled :P).
There was a problem hiding this comment.
I don't know of a way to check for unconditionally panic-ing too-generic code, other than things like the unconditional panic lint which I would have expected to catch something like this? 🤔
I think the only principled way to avoid this is too not support generic default values but that seems like it would kinda screw the usability of the feature 🤔
There was a problem hiding this comment.
Cop-out solution: Explicit lint on structs with generics and default field values. You must explicitly allow it as a signal that "yes, I am aware that these consts are only evaluated when used". It will make the feature more annoying to use because of attribute, but at least then it will be clear.
Edit: I just checked, and unconditionally doing const_eval_poly no longer ICEs when referencing a const param directly either.
It feels to me that we can do
if let Some(def_id) = field.value {
if let Err(ErrorHandled::TooGeneric(span)) = tcx.const_eval_poly(def_id) {
let field_span = tcx.def_span(def_id);
let mut multispan: MultiSpan =
if span.is_dummy() { field_span.into() } else { span.into() };
multispan.push_span_label(field_span, "unevaluated default value");
multispan.push_span_context(tcx.def_span(field.did));
multispan.push_span_context(tcx.def_span(item).shrink_to_lo());
tcx.dcx().span_warn( // FIXME: turn into a lint
multispan,
format!(
"field `{}` has a default value that is only checked when a value \
of `{}` is constructed",
field.name,
tcx.def_path_str(item),
),
);
}
}and get pretty much the behavior we'd want:
warning: field `field` has a default value that is only checked when a value of `Container` is constructed
--> y.rs:14:9
|
10 | struct Container<const N: usize> {
11 | field:
12 | ()
13 | = {
| _______-
14 | | f::<N>();
| | ^^^^^^^^
15 | | panic!();
16 | | },
| |_____- unevaluated default value
= note: lint `...` enabled by default
help: if this behavior is acceptable, allow the lint and preferably write a test relying on the default value
|
10 + #[allow(..)]
11 | struct Container<const N: usize> {
|
What do people think?
(I'd love it if we improved TooGeneric to carry an appropriate span more often than it does today.)
There was a problem hiding this comment.
fmease
added
S-blocked
Lint default field values in types with type or const parameters as they won't be evaluated pre-mono
Default field values in types that have const generics are no longer being evaluated until mono. Emit a warn-by-default lint so that API designers are not caught of guard by this behavior.
```
warning: field `multiline_field` has a default value that is only checked when a value of `Z` is constructed
--> $DIR/field-references-param-accurate-span.rs:9:15
|
LL | struct Z<const X: usize> {
LL | multiline_field:
LL | ()
LL | = {
| _______________^
LL | | f::<X>();
LL | | panic!();
LL | | },
| |_____________^ this can't be const-evaluated until use
|
= help: structs and enums with type and const parameters only evaluate their default field values during construction, not eagerly when declared
= note: `#[warn(unevaluated_default_field_value)]` on by default
help: if this behavior is acceptable, allow the lint and preferably write a test relying on the default value
|
LL + #[expect(unevaluated_default_field_value)]
LL | struct Z<const X: usize> {
|
```
Support `Span` context in lints.
Fixes #146496.
Part of #132162.
Alternative to #163182.
CC @fmease @BoxyUwU
|
☔ The latest upstream changes (presumably #163235) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
|
Superseded by #163235. |
Lint default field values in types with type or const parameters as they won't be evaluated pre-mono
Default field values in types that have const generics are no longer being evaluated until mono. Emit a warn-by-default lint so that API designers are not caught of guard by this behavior.
```
warning: field `multiline_field` has a default value that is only checked when a value of `Z` is constructed
--> $DIR/field-references-param-accurate-span.rs:9:15
|
LL | struct Z<const X: usize> {
LL | multiline_field:
LL | ()
LL | = {
| _______________^
LL | | f::<X>();
LL | | panic!();
LL | | },
| |_____________^ this can't be const-evaluated until use
|
= help: structs and enums with type and const parameters only evaluate their default field values during construction, not eagerly when declared
= note: `#[warn(unevaluated_default_field_value)]` on by default
help: if this behavior is acceptable, allow the lint and preferably write a test relying on the default value
|
LL + #[expect(unevaluated_default_field_value)]
LL | struct Z<const X: usize> {
|
```
Support `Span` context in lints.
Fixes rust-lang/rust#146496.
Part of rust-lang/rust#132162.
Alternative to rust-lang/rust#163182.
CC @fmease @BoxyUwU
5 participants
This is the solution/approach I've proposed in #146496 (and which I currently use for free GCIs). While I did ask T-types in #t-types/nominated > #146496: The way `default_field_values` deals with in-scope… whether they were okay with these exact semantics, I don't think I got a clear response to that, so I'm assigning a T-types member.
Fixes #146496.
Part of #132162.
Re. draft status: FIXME: Add more+better tests & add explainers.
cc @estebank
r? types
(No LLM was or will be used by me during the entire creation process of this PR)