Skip to content

Dealing with type/const ambiguities #480

Closed
@lcnr

Description

@lcnr

Proposal

It is not always possible to check whether a given generic argument should be a type or a const until typeck. While this is mostly fine, this most makes it difficult to represent ambiguous paths and _ in the hir.

Examples

This is the behavior on stable:

// When encountering ambiguity, we prefer types over constants,
// requiring the use of braces `{ N }` to interpret
// things as constants in case an identifier exists in
// both the type and the value namespace.
//
// `_` is forbidden as a const argument for now.
enum N { Variant };
struct Foo<const N: usize>;
fn foo<const N: usize>() -> Foo<N> {
    //~^ ERROR type provided when a constant was expected
    Foo
}

const N: usize = 3;
type Bar = Foo<N>; 
//~^ ERROR type provied when a constant was expected

fn main() {
    let _: Foo<3> = foo::<_>();
    //~^ ERROR type provided when a constant was expected
}

As we want to be able to resolve that ambiguity and get the above example to compile, we need a way to represent ambiguous generic arguments in the hir (and to a lesser degree in the ast). This representation should ideally avoid multiple equivalent representation and minimize the risk of misuse.

This MCP only cares about the hir for now as we don't really use the ast for anything which cares about types or constants.

Suggested solution

Modify the hir AST to the following

enum SharedTyConst<'hir> {
    Infer(...), // `_`
    Path(...),
}

enum TyKind<'hir> {
    Slice(&'hir Ty<'hir>),
    Array(&'hir Ty<'hir>, ArrayLen),
    // ...all existing variants, except for `Infer` and `Path`
}

enum ArrayLen<'hir> {
    Shared(SharedTyConst<'hir>),
    Body(AnonConst),
}

// Probably need a different name for this
struct Ty<'hir> {
    hir_id: HirId,
    kind: TyKind<'hir>,
    span: Span,
}

// This one is new, for type not directly used as a generic arg.
enum BikeshedTy {
    Shared(SharedTyConst<'hir>),
    Type(Ty<'hir>),
}

pub enum GenericArg<'hir> {
    Lifetime(Lifetime),
    Type(Ty<'hir>),
     // ConstArg is `AnonConst + Span`, mostly equiv to `ArrayLen::Body`.
    Const(ConstArg),
    Ambiguous(SharedTyConst<'hir>),
}

With this setup, every type and constant can only be represented one way.

How hir::intravisit::Visitor should now be implemented to avoid bugs while still being fairly usable.

enum SharedContext {
    FreeType,
    GenericArg,
    ArrayLength,
}

trait Visitor {
    fn visit_ty(&mut self, t: &'v Ty<'v>);
    fn visit_anon_const(&mut self, ct: AnonConst);
    fn visit_shared_ty_const(
        &mut self,
        shared: SharedTyConst<'hir>,
        context: SharedContext,
    )
    
    // The rest remains the same.
}

// calls `visit_ty` or `visit_shared_ty_const`.
fn walk_bikeshed_ty(visitor, bikeshed_ty) { ... }
fn walk_array_len(visitor, array_len) { ... }

While we need to add comments to visit_ty on how to deal with _ and paths, you can't really misuse that as these variants just don't exist.

Another issue is what happens if we want to get a ty::Ty for some hir::Ty. Afaict the only hir visitor which does that is HirWfCheck which I intend to manually update to also consider SharedTyConst. I am not sure if there's a good way to prevent us from forgetting this in the general case.

Sidenote: Paths and nameres

Not yet sure how to best deal with the ambiguity for paths during nameres, maybe by adding a variant Res::ForGenericArg(Box<[Res; 2]>) or something? 🤷

Mentors or Reviewers

If you have a reviewer or mentor in mind for this work, mention then
here. You can put your own name here if you are planning to mentor the
work.

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-compilerAdd this label so rfcbot knows to poll the compiler teammajor-changeA proposal to make a major change to rustc

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions