Repository navigation
The way default_field_values deals with in-scope generic parameters is slightly unprincipled #146496
Description
During wfcheck we currently const_eval_poly default field values unless its ascribed type references generic parameters. However, this doesn't take into account generic parameters referenced by the const expr (this matters because the final const value or its type doesn't necessarily need to reference them).
This is problematic as const-eval bails out early with TooGeneric if it (in some underspecified(?) value-based fashion) encounters a type or const param meaning it does not report an error! This leads to surprising (and imo unwanted) behavior. For example, under the current implementation downstream can break if an upstream const fn stops using a type or const param in its implementation (something you wouldn't think should affect callers I'd say):
//- upstream -//
pub const fn f<const N: usize>() {
let _ = [0u8; N]; // <-- comment out this line to break downstream!
}
//- downstream -//
#![feature(default_field_values)]
struct Container<const N: usize> {
field: () = {
upstream::f::<N>();
panic!();
},
}While const-evaluating the default value of Container.field, if the compiler encounters const param N "in a const value" (so e.g., let _: [u8; N]; wouldn't count) it'll silently bail out, not const-panic and thus succeed the build, otherwise it'll reach & execute the panic!(); and therefore fail the build pre-mono.
Alternative example involving a type param instead of a const param
//- upstream -//
pub trait Test { const TEST: bool; } // or in some other crate further upstream
pub const fn f<T: Test>() {
_ = T::TEST; // <-- comment out this line to break downstream!
}
//- downstream -//
#![feature(default_field_values)]
struct Container<T: upstream::Test> {
field: () = {
upstream::f::<T>();
panic!()
},
_marker: std::marker::PhantomData<T>,
}Of course, the final value of public const items, const fns and the like can hardly be considered a private implementation detail (and modifying it would require a major version bump strictly speaking). However, this is about the const expr, the unevaluated body, where I'd hope there to be a bit more wiggle room.
Regarding solutions, I see two obvious ones: Either
- reject const exprs that reference generic params (that's the approach we take for enum discriminant values) or
- don't evaluate any default field values (at the def-site1) if the enclosing ADT has
- any generic parameters or alternatively
- any "generic params that require monomorphization" (i.e., type and const params but not lifetime ones)
- that's the approach we currently take for free generic const items (feature:
generic_const_items) - since const-eval works modulo non-late-bound regions (they are erased just like during codegen), it's safe to eval the expr even if it references lifetime parameters
- if we end up following this approach, then we should probably also relax the rule "don't eval if the ascribed field type contains generic params" to "[…] type/const params" and eval the initializer in
struct X<'a> { x: &'a () = panic!() }
- that's the approach we currently take for free generic const items (feature:
Footnotes
-
At most use-sites we would still always evaluate as per the usual rules. ↩
Activity
fmease commentedon Sep 13, 2025
cc @estebank
fmease commentedon Sep 13, 2025
In my opinion this is a bug but it's possible that other project members disagree.
estebank commentedon Sep 15, 2025
I feel 1) might be too constraining, but 2) increase the complexity of the feature semantics and impl. I think 2) is the best choice.
estebank commentedon Mar 13, 2026
If I interpret correctly what was desired of 2), then changing this line to be
tcx.is_trivial_const(def_id)would do what you want. Note that we have testtests/ui/structs/default-field-values/invalid-const.rswhich would now start compiling (because none of those consts would be evaluated until the struct is constructed). I'd still want obviously failing consts like those to emit an error on the definition, somehow.fmease commentedon Mar 14, 2026
fmease commentedon Mar 14, 2026
is_trivial_constis used (and only to be used) for compiletime optimization purposes. We should definitely not tie it to program analysis / correctness. Its implementation may change at any point.fmease commentedon Mar 14, 2026
Right, well. This would need some kind of custom const expr visitor which doesn't walk into non-local functions etc1, hmm. I can understand why you'd want
panic!()to get evaluated even if type&const params are in scope as long as they're "not used in the expr" according to some more conservative metric.I don't want to make the call here. I'd rather WG-const-eval, T-lang and/or T-types(?) do that.
I'm just saying that I've made rustc not evaluate unstable GCI
const _<const N: usize>: () = panic!();to be consistent withfn f<const N: usize>() { const { panic!(); } }not getting evaluated (post-mono) due to that in-scope const param. It's also a lot more robust + simpler to implement.Footnotes
Even then, it wouldn't solve the analogous problem with non-local macros (however that might be considered negligible / could be exempt from stability guarantees). ↩
bstrie commentedon Aug 20, 2026
Given that this issue appears to currently be the only listed blocker for default field values, would someone like to nominate this for discussion?
oli-obk commentedon Sep 22, 2026
As discussed in https://rust-lang.zulipchat.com/#narrow/channel/326866-t-types.2Fnominated/topic/.23146496.3A.20The.20way.20.60default_field_values.60.20deals.20with.20in-scope.E2.80.A6 this is a bug and should be fixed.
estebank commentedon Sep 22, 2026
I would appreciate if someone from @rust-lang/wg-const-eval could take the lead on this change. I would like to stabilize the feature before EoY and I think this is the only outstanding issue and I want to make sure that it is done in a way that is acceptable to the WG.
RalfJung commentedon Sep 22, 2026
Cc @rust-lang/types
fmease commentedon Sep 22, 2026
While I did get a confirmation from T-types in the linked Zulip topic that this was a bug, in my eyes I didn't get a confirmation that they were also fine with the new semantics I've proposed. I've therefore gone ahead & opened PR #163182 with the semantics I desire to start a concrete discussion.
Auto merge of #163235 - estebank:default-field-values-lint, r=fmease
Auto merge of #163235 - estebank:default-field-values-lint, r=fmease