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: const functions in traits #3490

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Sep 18, 2023

Rendered

This RFC allows marking methods in traits as const.

trait Foo {
    const fn bar(a: i32) -> i32;
}

impl Foo for MyStruct {
    // Implementation must provide a const function
    const fn bar(a: i32) -> i32 {
        a + 100
    }
}

// `<T as Foo>::bar` can then be used in const contexts
const fn use_foo<T: Foo>(a: i32) -> i32 {
    T::bar(a) * 50
}

@tgross35
Copy link
Contributor Author

@rustbot label +T-lang

@rustbot
Copy link
Collaborator

rustbot commented Sep 18, 2023

Error: Label T-lang can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@compiler-errors compiler-errors added the T-lang Relevant to the language team, which will review and decide on the RFC. label Sep 18, 2023
@tgross35
Copy link
Contributor Author

cc @rust-lang/wg-const-eval

@clarfonthey
Copy link
Contributor

clarfonthey commented Sep 18, 2023

EDIT: In hindsight, I realise that I made this while tired and not fully understanding what this RFC is suggesting.

Going to effectively retract my comments here, but will leave it for posterity.


Going to be rather blunt: have you been following the development of const-fn-in-trait at all?

Because, this is a planned goal, but one of the reasons why keyword generics exist in the first place is to solve this problem. This RFC strikes me as being written by someone who hasn't followed that development, and just wants const trait support sooner. The problem is that we really can't offer it much sooner than finishing up the existing proposals.

Sure, we could tag some functions as const in traits, require that said functions are always const, then let you use those in const functions since the "constness" of the methods is never in question. But that's an incredibly restrictive constraint, and it doesn't really gain us much beyond our current system. A lot of the actual functionality folks are reaching for can be achieved with the current const fn system via hacks, and the stuff that can't won't really be helped by this change. All it seems like is a stopgap solution that will not really help much, and just delay the existing const trait effort further.

@tgross35
Copy link
Contributor Author

Going to be rather blunt: have you been following the development of const-fn-in-trait at all?

I am not familiar with anything specifically called const-fn-in-trait, nor can I find any specific information on it. If this isn't the same as keyword-generics-initiative/effects/const_generic_const_fn_bounds, do you have a link? Otherwise, I believe I am fairly up to date with what is publicly available.

Sure, we could tag some functions as const in traits, require that said functions are always const, then let you use those in const functions since the "constness" of the methods is never in question. But that's an incredibly restrictive constraint, and it doesn't really gain us much beyond our current system. A lot of the actual functionality folks are reaching for can be achieved with the current const fn system via hacks, and the stuff that can't won't really be helped by this change. All it seems like is a stopgap solution that will not really help much, and just delay the existing const trait effort further.

It is true that this RFC is encompassed by the effects goals. But effects are still very much in the experimentation stage and have a much larger and more complex scope (I have to assume we are still many months away from RFCs), so it did not seem there would be any harm in proposing a minor and fairly unobjectionable subset at this time. I also did some due diligence to ensure this wouldn't conflict 1 2.

I also have to disagree that this isn't useful on its own, I come across uses quite frequently in projects that use a lot of statics (e.g. modular C+Rust projects).

@programmerjake
Copy link
Member

programmerjake commented Sep 18, 2023

I also have to disagree that this isn't useful on its own, I come across uses quite frequently in projects that use a lot of statics (e.g. modular C+Rust projects).

I know I definitely will use this, currently I have to make-do with a bunch of consts in traits and other annoying workarounds. I even considered building an interpreter to take a const from a trait and treat it as code that can be run in CTFE.

My motivating use-case is compile-time checking of properties of a domain-specific language embedded in Rust.

eopb added a commit to eopb/rfcs that referenced this pull request Sep 20, 2023
@Matthew-Chidlow
Copy link

While some form of optional const-ness is needed in traits, regardless of how that is done, always const functions are something that you should be able to do. Since a trait is a contract and so its natural to be able to add const-ness to that contract, and this is exactly what has been done for async, you can add async to the contract to force a function to always be async so extending that to const is only natural.

Plus I would definitely use this if it existed.

@bluebear94
Copy link

Would refinining a trait method by adding const be a further possibility for this RFC?

@tgross35
Copy link
Contributor Author

Would refinining a trait method by adding const be a further possibility for this RFC?

I don't intend to cover allowing const implementations where the trait does not require const, just to keep this RFC scoped to the most basic need. However, that is actually called out in the refine RFC as a possibility and I think it would interact well with this.

Thanks for bringing it up, I added that under future possibilities.

@matthieu-m
Copy link

I'd like to share a concrete usecase for this feature.

In the RFC: Introduce the Store API for great good the Store trait has been artificially split into StoreDangling which exposes a single dangling method and Store itself which exposes the rest of the methods just so that dangling handles can be produced in const contexts -- and thus Vec::new and al. can be const -- without requiring that the entire Store implementation be const (hint: most won't be).

If this RFC were to be accepted, the Store API could be simplified to a single Store trait, with a required const fn dangling(...) method, and everyone would rejoice.

Comment on lines +30 to +31
Workarounds typically involve a combination of wrapper functions, macros, and
associated consts. This RFC will eliminate the need for such workarounds.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel it may be useful to note some of these workarounds?

Essentially, I'm searching for an illustration of what is possible (although maybe hugely inconvenient and ugly) in current Rust, and what new possibilities would be enabled by this proposal, if any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean that you are looking for a sample of what a solution might look like today? Or just a more in depth explanation from the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe just a better explanation of each workaround or the use cases would be best, current examples are usually pretty large and vary a lot

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, mostly I fear that there may be a lot of edge-cases that have not yet been considered in the proposal?

And then I think that if I saw a few instances of "this thing can already be done today, just not as cleanly", then it'd assuage my worries on that front.

text/0000-const-fn-in-trait.md Outdated Show resolved Hide resolved
/// Add a named item to our global state register
const fn register_state<T: GlobalState>(name: &'static str, item: T) {
// ...
STATES[0] = (name, item.build())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should provide some base_value to the build method

text/0000-const-fn-in-trait.md Outdated Show resolved Hide resolved
@BD103
Copy link

BD103 commented Sep 22, 2023

Just a small question: how would this work with trait objects? Will const functions still be turned into runtime functions when included in a VTable?

For instance:

trait Foo {
    const fn foo();
}

fn run_any_foo(trait_object: &dyn Foo) {
    trait_object.foo();
}

const fn run_any_foo(trait_object: &dyn Foo) {
    trait_object.foo();
}

If a const trait method is called from a const function, will it all be evaluated at compile time or raise an error? Will a const trait methods have to be included in the VTable, even if it is not necessarily called at runtime?

I don't have any specific ideas for changes to the RFC, I'm just making sure trait objects aren't forgotten. :)

tgross35 and others added 2 commits September 22, 2023 15:45
Co-authored-by: Mads Marquart <mads@marquart.dk>
Co-authored-by: Mads Marquart <mads@marquart.dk>
@tgross35
Copy link
Contributor Author

tgross35 commented Sep 22, 2023

Just a small question: how would this work with trait objects? Will const functions still be turned into runtime functions when included in a VTable?

[...]

If a const trait method is called from a const function, will it all be evaluated at compile time or raise an error? Will a const trait methods have to be included in the VTable, even if it is not necessarily called at runtime?

I don't have any specific ideas for changes to the RFC, I'm just making sure trait objects aren't forgotten. :)

That is a great question!

At runtime, const functions act identical to normal functions so I think it should be the same in traits - that is, the functions will be in the VTable. I am not sure what is best with respect to calling const-fn-in-trait functions from a &dyn Foo - it seems doable at first glance but because that may come with hidden trickiness.

I think maybe it is best to leave thta under unresolved questions or future possibilities for now, since more information will probably become apparent at implementation time. I'll update the RFC soon

@programmerjake
Copy link
Member

I think maybe it is best to leave thta under unresolved questions or future possibilities for now, since more information will probably become apparent at implementation time. I'll update the RFC soon

I think that can be left for when we get function pointers with const in the type, which is presumably what the methods will essentially be:

pub const fn call_it(f: const fn() -> i32) -> i32 {
    f()
}

@clarfonthey
Copy link
Contributor

Responding after re-reading the RFC, understanding for real what's actually going on. I've marked my previous comment to clarify that I really wasn't responding properly to this RFC, although I don't like deleting things since it makes discussions confusing to read.

I agree that having const fns in traits which are always const, regardless of implementer, is reasonable. The main issue is that semantically, it's weird to distinguish between a trait method which can opt into always being meaningful in a const context, versus one which must be const.

The reason why I had such a knee-jerk reaction to the RFC is because, well… there's the issue of potentially abusing this feature until keyword generics are released, where people start offering const and non-const versions of traits. This will just make migrating back into keyword generics a headache, although I agree that simply not having functionality in the meantime is a bad solution.

Overall it's just really confusing to reason about const because it's effectively backwards what it should be. Since something applicable at const time is also applicable at runtime, we should instead have a nonconst keyword that indicates when something it not available at const time, which would make the logic around other keywords (like mut and unsafe) more consistent with const. Of course, that's not what we have now, and we're stuck with it.

Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

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

I think the main issue with this is that of technical debt. If we ever wanted traits like Iterators to be const and usable in const contexts, then we need to introduce constness to those traits in a backwards-compatible way. This RFC cannot help with eventually making Iterator const, since adding const to a trait function would be a breaking change. This means that a future RFC that proposes "const traits", like the experimental feature we currently have under const_trait_impl and effects will eventually need to be proposed.

But, if we allow the ability to have const functions in traits, we are suggesting traits like ConstAdd or ConstIterator so that someone can use those traits in const functions with generic parameters. Perhaps this works fine when someone is implementing something like a generic factorial function and all the types they are handling can have subtraction and multiplication done in compile time, but this potentially creates fragmentation where non-const functions are unable to call generic const functions due to the strict trait impl requirements. If we are pushing to stabilize this faster than const traits, then this would be concerning.

Also, cc @rust-lang/wg-const-eval since I didn't get notified for the first ping it seems


Adding this feature approaches the goal of "Rust working like one would expect
it to work". That is, functions in traits generally act similar to functions
outside of traits, and this feature serves to bridge one of the final gaps
Copy link
Member

Choose a reason for hiding this comment

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

looks like you didn't finish this sentence here

Comment on lines +137 to +138
This feature requires tracking more information related to trait definition and
usage, but this should be a relatively maintenance burden.
Copy link
Member

Choose a reason for hiding this comment

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

relatively low?

Comment on lines +89 to +91
standard `const` functions when needed at CTFE. This additional metadata can be
stripped in all other cases and the function will act the same as a non-const
function.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that is what we do with const functions? IIRC marking a function has const can change inliner behavior, but I might just be misremembering.

- Reducing code duplication in const contexts
- Providing a constructor for generic static object, e.g. C-style plugin
registration
- Subtraits that want to provide defaults based on supertraits
Copy link
Member

@fee1-dead fee1-dead Oct 6, 2023

Choose a reason for hiding this comment

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

I'm not sure what this use case is and why it relates to const functions in traits, could you specify?

Comment on lines +124 to +126
const<C> trait SometimesConstFoo {
const <C> fn sometimes_const_foo(&self) -> u32
}
Copy link
Member

Choose a reason for hiding this comment

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

This is not equivalent to const functions in traits, because there is no parameterization based on constness on the trait. Under effects, const functions in traits will become generic over constness itself, but not the traits (but we still treat it as a non-generic in different parts of rustc, something like foo<'a>)

@oli-obk
Copy link
Contributor

oli-obk commented Oct 6, 2023

I would prefer to postpone this RFC until we have a good implementation of const traits. Landing this RFC would suggest that we'd also work on it, and I don't think we can do that until const the const traits work settles down, as it's already hard enough to keep one major const feature implementation in mind.

@tgross35
Copy link
Contributor Author

tgross35 commented Oct 6, 2023

Thanks for taking a look! With the potential conflicting work in mind, I don't see this going anywhere anytime soon. I'll leave it around for now though as a draft just to continue collecting any feedback

@tgross35 tgross35 marked this pull request as draft October 6, 2023 17:25
@matthieu-m
Copy link

This RFC cannot help with eventually making Iterator const, since adding const to a trait function would be a breaking change.

Indeed, it cannot, but I do not see this as a problem.

We can today have inherent impl blocks which mix-and-match const, async, and neither-const-nor-async associated functions, and arguably we need the same for traits.

I already mentioned the usecase of the Store API. Today, in order to produce a dangling handle in const contexts, which is necessary for a const Vec::new, a separate trait had to be introduced for the one function that requires to be const. It's workable, but not ergonomic.

The clearly better API is for the Store trait to be implemented as both const and non-const -- depending on what's possible -- and for its dangling method to always be const.

In the end, we need the flexibility of both const functions in traits and const traits, neither one subsumes the other, they are just appropriate in different circumstances.

I would prefer to postpone this RFC until we have a good implementation of const traits. Landing this RFC would suggest that we'd also work on it, and I don't think we can do that until const the const traits work settles down, as it's already hard enough to keep one major const feature implementation in mind.

I'm ambivalent on this.

I understand the motivation, however it may also be worth accepting such an RFC to signal that const traits will not be the end of the story, and developers for which const methods in traits would be a better fit may want to wait for it, rather than stabilize work-arounds based on const traits.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 7, 2023

I understand the motivation, however it may also be worth accepting such an RFC to signal that const traits will not be the end of the story,

We don't even have an accepted const trait RFC 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet