rust-lang / rust-lang/rust

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

Open
#146,496 8 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

A-const-eval C-bug F-default_field_values I-types-nominated T-compiler T-types
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

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

  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-site[^1]) 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!() }

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in wfcheck at the default_field_values path and trace its use of const_eval_poly, focusing on how generic parameters in the expression differ from those in the ascribed type. Decide between rejecting generic-dependent expressions and deferring evaluation, then verify the chosen behavior against the upstream/downstream const- and type-parameter examples.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.