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

Allow using generic trait methods in const fn #79287

Merged
merged 4 commits into from
Nov 23, 2020
Merged

Allow using generic trait methods in const fn #79287

merged 4 commits into from
Nov 23, 2020

Conversation

jonas-schievink
Copy link
Contributor

Next step for #67792, this now also allows code like the following:

struct S;

impl const PartialEq for S {
    fn eq(&self, _: &S) -> bool {
        true
    }
}

const fn equals_self<T: PartialEq>(t: &T) -> bool {
    *t == *t
}

pub const EQ: bool = equals_self(&S);

This works by threading const-ness of trait predicates through trait selection, in particular through ParamCandidate, and exposing it in the resulting ImplSource.

Since this change makes two bounds T: Trait and T: ?const Trait that only differ in their const-ness be treated like different bounds, candidate winnowing has been changed to drop the ?const candidate in favor of the const candidate, to avoid ambiguities when both a const and a non-const bound is present.

@rust-highfive
Copy link
Collaborator

r? @oli-obk

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 22, 2020
@jyn514 jyn514 added A-const-fn A-trait-system Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 22, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2020

This is absolutely great! A few points for clarity:

  • Please add tests for mixes of ?const Trait bounds and Trait bounds, where there would be ambiguities for figuring out which bound should be called.
  • Please add a test for calling a generic method with bounds from withing a generic function with bounds. Ideally different tests for both with ?const and without on the method's bounds
    • or will this just hit the same issue as src/test/ui/rfc-2632-const-trait-impl/call-generic-method-nonconst.rs?
  • Track the problem about the actual usage within Tracking issue for RFC 2632, impl const Trait for Ty and ~const (tilde const) syntax #67792

@jonas-schievink
Copy link
Contributor Author

I've added more tests, but I'm not sure what you mean by this:

Track the problem about the actual usage within #67792

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2020

I added some text about the issue of using types that do not impl const Trait for const fn parameters that have Trait bounds and thus expect const Trait impls.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 22, 2020

📌 Commit cb40684 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2020
@RalfJung
Copy link
Member

Do we have tests to ensure that this only works when a feature flag is set?

@oli-obk
Copy link
Contributor

oli-obk commented Nov 22, 2020

@jonas-schievink
Copy link
Contributor Author

There are no const impls in libcore/std, so using one currently also requires defining one. I'll add a note to the tracking issue.

m-ou-se added a commit to m-ou-se/rust that referenced this pull request Nov 22, 2020
…=oli-obk

Allow using generic trait methods in `const fn`

Next step for rust-lang#67792, this now also allows code like the following:

```rust
struct S;

impl const PartialEq for S {
    fn eq(&self, _: &S) -> bool {
        true
    }
}

const fn equals_self<T: PartialEq>(t: &T) -> bool {
    *t == *t
}

pub const EQ: bool = equals_self(&S);
```

This works by threading const-ness of trait predicates through trait selection, in particular through `ParamCandidate`, and exposing it in the resulting `ImplSource`.

Since this change makes two bounds `T: Trait` and `T: ?const Trait` that only differ in their const-ness be treated like different bounds, candidate winnowing has been changed to drop the `?const` candidate in favor of the const candidate, to avoid ambiguities when both a const and a non-const bound is present.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 23, 2020
…as-schievink

Rollup of 10 pull requests

Successful merges:

 - rust-lang#76829 (stabilize const_int_pow)
 - rust-lang#79080 (MIR visitor: Don't treat debuginfo field access as a use of the struct)
 - rust-lang#79236 (const_generics: assert resolve hack causes an error)
 - rust-lang#79287 (Allow using generic trait methods in `const fn`)
 - rust-lang#79324 (Use Option::and_then instead of open-coding it)
 - rust-lang#79325 (Reduce boilerplate with the `?` operator)
 - rust-lang#79330 (Fix typo in comment)
 - rust-lang#79333 (doc typo)
 - rust-lang#79337 (Use Option::map instead of open coding it)
 - rust-lang#79343 (Add my (`@flip1995)` work mail to the mailmap)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c7a6720 into rust-lang:master Nov 23, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 23, 2020
@jonas-schievink jonas-schievink deleted the const-trait-impl branch November 23, 2020 23:09
@leonardo-m
Copy link

I think if you write the keywords in the wrong order "const impl PartialEq for S {" the compiler could give a bit better error message.

@leonardo-m
Copy link

See also: #79378

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 13, 2020
…trochenkov

Recover on `const impl<> X for Y`

`@leonardo-m` mentioned that `const impl Foo for Bar` could be recovered from in rust-lang#79287.

I'm not sure about the error strings as they are, I think it should probably be something like the error that `expected_one_of_not_found` makes + the suggestion to flip the keywords, but I'm not sure how exactly to do that. Also, I decided not to try to handle `const unsafe impl` or `unsafe const impl` cause I figured that `unsafe impl const` would be pretty rare anyway (if it's even valid?), and it wouldn't be worth making the code more messy.
@usbalbin
Copy link
Contributor

Is something like this supposed to work with this PR? Or is that something which might get implemented in a later stage? :)

impl<T, U> const Into<U> for T
where
    U: From<T>,
{
    fn into(self) -> U {
        U::from(self)
    }
}

I currently get this error

error[E0015]: calls in constant functions are limited to constant functions, tuple structs and tuple variants

@oli-obk
Copy link
Contributor

oli-obk commented Dec 26, 2020

This is unimplemented as of now, but being looked into since around a week

@usbalbin
Copy link
Contributor

Oh, sorry. Thanks :)

@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-trait-system Area: Trait system S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants