Skip to content

The way default_field_values deals with in-scope generic parameters is slightly unprincipled #146496

@fmease

Description

@fmease

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

  1. reject const exprs that reference generic params (that's the approach we take for enum discriminant values) or
  2. don't evaluate any default field values (at the def-site1) if the enclosing ADT has
    1. any generic parameters or alternatively
    2. 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!() }

Footnotes

  1. At most use-sites we would still always evaluate as per the usual rules.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-const-evalArea: Constant evaluation, covers all const contexts (static, const fn, ...)C-bugCategory: This is a bug.F-default_field_values`#![feature(default_field_values)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions