Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Associated const underscore #3527

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Nov 12, 2023

Allow _ for the name of associated constants. This RFC builds on RFC 2526 which added support for free const items with the name _, but not associated consts.

// RFC 2526 (stable in Rust 1.37)
const _: () = { /* ... */ };

impl Thing {
    // this RFC
    const _: () = { /* ... */ };
}

Constants named _ are not nameable by other code and do not appear in documentation, but are useful when macro-generated code must typecheck some expression in the context of a specific choice of Self.

Rendered

@dtolnay dtolnay added T-lang Relevant to the language team, which will review and decide on the RFC. A-syntax Syntax related proposals & ideas A-const Proposals relating to const items labels Nov 12, 2023
@dtolnay dtolnay force-pushed the assocunderscore branch 2 times, most recently from 73c9d1f to 9db99f3 Compare November 12, 2023 21:22
@programmerjake
Copy link
Member

When does associated const code get run? at type definition? when substituting concrete types into generic arguments? never?
so, if I write:

pub struct S<T>(T);

impl<T> S<T> {
    const _: () = assert!(core::mem::size_of::<T>() % 2 == 0);
}

when does the assert get run?

@dtolnay
Copy link
Member Author

dtolnay commented Nov 13, 2023

Great question! I had not thought about that. I have added a paragraph proposing that they are evaluated never, aligning with the behavior of a named associated constant that is accessed never, but I'd welcome considering other options.

@programmerjake
Copy link
Member

programmerjake commented Nov 13, 2023

I have added a paragraph proposing that they are evaluated never, aligning with the behavior of a named associated constant that is accessed never, but I'd welcome considering other options.

I think that never being evaluated will be highly unintuitive, we should instead require them to be evaluated (though memoization is allowed) when generating any code that creates a type Self where enough generics have concrete types/values such that all generics used in the const are concrete types/values.

so:

pub struct S<T, U, const V: usize>(T, U);

impl<T, U, const V: usize> S<T, U, V> {
    const _: () = {
        let v: Option<T> = None; // use `T`
        panic!("1: T");
    };
    const _: () = {
        let v: Option<(T, U)> = None; // use `T` and `U`
        panic!("2: T and U");
    };
    const _: () = {
        let v: Option<(T, U)> = None; // use `T` and `U`
        let v = [(); V]; // use `V`
        panic!("3: T, U, and V");
    };
}

pub fn f<T, U>(v: S<T, U, 8>>) {} // doesn't error, since only V has a concrete value
pub fn g<U, const V: usize>(v: S<i8, U, V>>) {} // errors with message "1: T"
pub fn h<const V: usize>() {
    let f = f::<u8, i8, V>; // errors with messages "1: T" and "2: T and U"
}
const _: () = {
    let v: S<u8, i8, 5>; // errors with messages "1: T" and "2: T and U" and "3: T, U, and V"
};

@programmerjake
Copy link
Member

programmerjake commented Nov 13, 2023

so, basically as if:

pub struct S<T, U, const V: usize>(T, U);

impl<T, U, const V: usize> S<T, U, V> {
    const _: () = ...;
}

was instead written:

pub struct S<T, U, const V: usize>(T, U, PhantomData<[(); {Self::_CONST; 0}]>);

impl<T, U, const V: usize> S<T, U, V> {
    const _CONST: () = ...;
}

@joshlf
Copy link
Contributor

joshlf commented Nov 16, 2023

This would be so helpful! Thanks for writing this, David!

In favor of these constants being evaluated: rust-lang/rust#112090

@trueb2
Copy link

trueb2 commented Nov 16, 2023

It seems that the intent of the change is to allow undocumented static assertions within traits, where this is easier to implement. There are probably corner cases that I cannot think of immediately. I found the use of anonymous const blocks non-intuitive on first encounter but clearly appreciate their use now.

I would expect code in const blocks to only affect compilation; therefore, they are fundamentally different than creating symmetry to let bindings.

I also anticipate a natural progression that if all of the const blocks symmetry is added, they will be sugared out by keyword or macro. Like C++ static_assert or Zig comptime where intent is clear/concise.

@scottmcm
Copy link
Member

scottmcm commented Nov 18, 2023

I was all set to propose merge as "of course" until I saw the note about evaluation :/

I do agree that not evaluating it is consistent with named associated constants.

Unfortunately, I feel like it would be particularly likely for someone to move a const _: () = assert!(...); into an associated constant and not notice that it's just never running. At least with a named one you get an error if it's never used, and adding a use somewhere gives an opportunity to evaluate it when instantiated.

Brainstorming: How much would checks here be needed to put into inherent impls vs wanting them only in trait impl blocks? For example, I'm pondering an alternative where you'd expand derive(Eq) to

impl<T> ::core::cmp::Eq for Thing<'a, T> {
    #[coverage(off)]
    priv fn _assert_fields_are_total_eq<'a, T: ::core::cmp::Eq>() {
        let _: ::core::cmp::AssertParamIsEq<Field<'a, T>>;
    }
}

even though there's no _assert_fields_are_total_eq on Eq, assuming a lang change where you're allowed to put extra items in impl blocks so long as they're priv (placeholder, but it's already a keyword, so that's convenient), which makes them visible inside that impl block only. (No need for doc(hidden) on it because it'd be treated the same as other non-pub things and only included in docs with --document-private-items.)

Being a function means the fact that it's not ever evaluated seem more normal, and helper functions in impls that don't have to be put in separate inherent impl blocks seems like an independently-useful feature too. (But also a bigger one, because anything that deals in visibility is complicated.)

@nikomatsakis
Copy link
Contributor

One further thought: the whole const _: () = { .. } thing was a total hack -- albeit, a useful one. I do wonder if it's time to add something more first-class, e.g. "anonymous modules", something like that.

@scottmcm scottmcm added the I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. label Nov 22, 2023
@traviscross
Copy link
Contributor

@rustbot labels -I-lang-nominated

We discussed this in the T-lang meeting last week.

There was consensus that the problem this is trying to solve is a real problem.

However, people were concerned about whether it might be too surprising that these were not evaluated. It wasn't that people wanted them evaluated exactly; the concern was simply the potential for surprise.

Aside from the practical motivation (which may be better served by some more specialized feature), the main reason to allow this (with the proposed behavior) would seem to be consistency on two fronts. We allow const _ elsewhere, and we don't evaluate unused associated constants.

The consensus was that we wanted more discussion in the RFC itself about this behavior, e.g. acknowledgment that this might be surprising, discussion of things we could do to mitigate the surprise, discussion of why maybe this would be less surprising than it seems or why the consequences of that might not be severe, an argument for why we should do it anyway, etc.

Please renominate for T-lang after addressing these in the RFC so the team can review again.

@rustbot rustbot removed the I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. label Dec 13, 2023
Comment on lines +546 to +547
# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider discussing the relationship with #3373 — probably just to confirm that associated const underscore is not exempt from that lint.

pub struct Struct;

const _: () = {
    impl Trait1 for Struct {}  // no warning
};

impl Struct {
    const _: () = {
        impl Trait2 for Struct {}  // WARNING
    };
}

@PoignardAzur
Copy link

PoignardAzur commented May 3, 2024

If the concern is about people being surprised by code not being evaluated, how about introducing wildcard associated functions?

Eg:

impl Thing {
    fn _() {
        /* ... */
    };
}

This would probably be more intuitive to most people. If someone reads this code:

impl Thing {
    fn _() {
        assert!(...);
    };
}

It's immediately obvious that something is wrong here because that function is obviously never going to be run.

@clarfonthey
Copy link
Contributor

Finding myself wanting this specifically for assertions: is there a particular reason why we couldn't change the associated type behaviour at an edition boundary, and then warn for associated-const-underscore usages on previous editions? Or is this not specific to associated constants and actually specific for all named constants, and it's just that we're wanting this in particular for the associated constants?

Right now I'm using them as a way of confirming validity of generic consts, since even simple things like casting a u8 to a usize so it can be used as an array length is not allowed. I have to accept a usize as my generic const, then panic if its length is more than 255, which I would prefer to guarantee statically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const Proposals relating to const items A-syntax Syntax related proposals & ideas T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants