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.

Sign up for GitHub

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

Jump to bottom

Check that const parameters of trait methods have compatible types #86843

Merged
merged 1 commit into from
Jul 18, 2021
Merged

Check that const parameters of trait methods have compatible types #86843

merged 1 commit into from
Jul 18, 2021

Conversation

FabianWolff
Copy link
Contributor

This PR fixes #86820. The problem is that this currently passes the type checker:

trait Tr {
    fn foo<const N: u8>(self) -> u8;
}

impl Tr for f32 {
    fn foo<const N: bool>(self) -> u8 { 42 }
}

i.e. the type checker fails to check whether const parameters in impl methods have the same type as the corresponding declaration in the trait. With my changes, I get, for the above code:

error[E0053]: method `foo` has an incompatible const parameter type for trait
 --> test.rs:6:18
  |
6 |     fn foo<const N: bool>(self) -> u8 { 42 }
  |                  ^
  |
note: the const parameter `N` has type `bool`, but the declaration in trait `Tr::foo` has type `u8`
 --> test.rs:2:18
  |
2 |     fn foo<const N: u8>(self) -> u8;
  |                  ^

error: aborting due to previous error

This fixes #86820, where an ICE happens later on because the trait method is declared with a const parameter of type u8, but the impl uses one of type usize:

expected int of size 8, but got size 1

@rust-highfive
Copy link
Collaborator

r? @LeSeulArtichaut

(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 Jul 3, 2021
@LeSeulArtichaut
Copy link
Contributor

Maybe r? @jackh726

lcnr
lcnr approved these changes Jul 14, 2021
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

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

this PR is looking good, some nits but after that r=me

compiler/rustc_typeck/src/check/compare_method.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/check/compare_method.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/check/compare_method.rs Outdated Show resolved Hide resolved
@lcnr
Copy link
Contributor

lcnr commented Jul 14, 2021

r? @lcnr

@rust-highfive rust-highfive assigned lcnr and unassigned jackh726 Jul 14, 2021
@lcnr
Copy link
Contributor

lcnr commented Jul 17, 2021

thx

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 17, 2021

📌 Commit 9b874c4 has been approved by lcnr

@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 Jul 17, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jul 17, 2021
@GuillaumeGomez
Rollup merge of rust-lang#86843 - FabianWolff:issue-86820, r=lcnr
821ae76 
Check that const parameters of trait methods have compatible types

This PR fixes rust-lang#86820. The problem is that this currently passes the type checker:
```rust
trait Tr {
    fn foo<const N: u8>(self) -> u8;
}

impl Tr for f32 {
    fn foo<const N: bool>(self) -> u8 { 42 }
}
```
i.e. the type checker fails to check whether const parameters in `impl` methods have the same type as the corresponding declaration in the trait. With my changes, I get, for the above code:
```
error[E0053]: method `foo` has an incompatible const parameter type for trait
 --> test.rs:6:18
  |
6 |     fn foo<const N: bool>(self) -> u8 { 42 }
  |                  ^
  |
note: the const parameter `N` has type `bool`, but the declaration in trait `Tr::foo` has type `u8`
 --> test.rs:2:18
  |
2 |     fn foo<const N: u8>(self) -> u8;
  |                  ^

error: aborting due to previous error
```
This fixes rust-lang#86820, where an ICE happens later on because the trait method is declared with a const parameter of type `u8`, but the `impl` uses one of type `usize`:
> `expected int of size 8, but got size 1`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 17, 2021
Closed
@JohnTitor JohnTitor mentioned this pull request Jul 18, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2021
@bors
Auto merge of rust-lang#87242 - JohnTitor:rollup-t9rmwpo, r=JohnTitor
5a8a441 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#86763 (Add a regression test for issue-63355)
 - rust-lang#86814 (Recover from a misplaced inner doc comment)
 - rust-lang#86843 (Check that const parameters of trait methods have compatible types)
 - rust-lang#86889 (rustdoc: Cleanup ExternalCrate)
 - rust-lang#87092 (Remove nondeterminism in multiple-definitions test)
 - rust-lang#87170 (Add diagnostic items for Clippy)
 - rust-lang#87183 (fix typo in compile_fail doctest)
 - rust-lang#87205 (rustc_middle: remove redundant clone)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 783efd2 into rust-lang:master Jul 18, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 18, 2021
@lcnr lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021
@lcnr lcnr mentioned this pull request Dec 11, 2021
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr approved these changes

Assignees

@lcnr lcnr

Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.

int.rs:230:13: expected int of size 8, but got size 1
7 participants
@FabianWolff @rust-highfive @LeSeulArtichaut @lcnr @bors @jackh726 @rustbot