Skip to content

Report an error if a lang item has the wrong number of generic arguments - #85339

Merged
bors merged 3 commits into
rust-lang:masterfrom
FabianWolff:issue-83893
May 18, 2021
Merged

Report an error if a lang item has the wrong number of generic arguments#85339
bors merged 3 commits into
rust-lang:masterfrom
FabianWolff:issue-83893

Conversation

@FabianWolff

@FabianWolffFabianWolff commented May 15, 2021

Copy link
Copy Markdown
Contributor

This pull request fixes#83893. The issue is that the lang item code currently checks whether the lang item has the correct item kind (e.g. a #[lang="add"] has to be a trait), but not whether the item has the correct number of generic arguments.

This can lead to an "index out of bounds" ICE when the compiler tries to create more substitutions than there are suitable types available (if the lang item was declared with too many generic arguments).

For instance, here is a reduced ("reduced" in the sense that it does not trigger additional errors) version of the example given in #83893:

#![feature(lang_items,no_core)]#![no_core]#![crate_type="lib"]#[lang = "sized"]traitMySized{}#[lang = "add"]traitMyAdd<'a,T>{}fnice(){let r = 5;let a = 6;
r + a
}

On current nightly, this immediately causes an ICE without any warnings or errors emitted. With the changes in this PR, however, I get no ICE and two errors:

error[E0718]: `add` language item must be applied to a trait with 1 generic argument
--> pr-ex.rs:8:1
|
8 | #[lang = "add"]
| ^^^^^^^^^^^^^^^
9 | trait MyAdd<'a, T> {}
| ------- this trait has 2 generic arguments, not 1
error[E0369]: cannot add `{integer}` to `{integer}`
--> pr-ex.rs:14:7
|
14 | r + a
| - ^ - {integer}
| |
| {integer}
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0369, E0718.
For more information about an error, try `rustc --explain E0369`.

@rust-highfive

rust-highfive commented May 15, 2021

Copy link
Copy Markdown
Contributor

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2021
varkor
varkor reviewed May 16, 2021
View reviewed changes

@varkorvarkor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks – this is looking pretty good. Just have a couple of comments.

Comment threadcompiler/rustc_passes/src/lang_items.rs Outdated
Comment threadcompiler/rustc_passes/src/lang_items.rs
Comment threadcompiler/rustc_typeck/src/check/op.rs Outdated
Comment threadcompiler/rustc_typeck/src/check/place_op.rs Outdated
Comment threadcompiler/rustc_passes/src/lang_items.rs Outdated
varkor
varkor reviewed May 17, 2021
View reviewed changes

@varkorvarkor left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you could revert the change in collect_item and address the comment in has_expected_num_generic_args, I think this is good to go.

Comment threadcompiler/rustc_typeck/src/check/mod.rs Outdated
@FabianWolff

FabianWolff commented May 17, 2021

Copy link
Copy Markdown
ContributorAuthor

If you could revert the change in collect_item and address the comment in has_expected_num_generic_args, I think this is good to go.

Done. Thanks for your suggestions and for taking the time to review this PR!

@varkor

varkor commented May 17, 2021

Copy link
Copy Markdown
Contributor

Thanks for fixing this, @FabianWolff!

@bors r+

@bors

bors commented May 17, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit 7b30198 has been approved by varkor

@borsbors 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 May 17, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 18, 2021
@GuillaumeGomez
Rollup merge ofrust-lang#85339- FabianWolff:issue-83893, r=varkor
d29a726
Report an error if a lang item has the wrong number of generic arguments
This pull request fixesrust-lang#83893. The issue is that the lang item code currently checks whether the lang item has the correct item kind (e.g. a `#[lang="add"]` has to be a trait), but not whether the item has the correct number of generic arguments.
This can lead to an "index out of bounds" ICE when the compiler tries to create more substitutions than there are suitable types available (if the lang item was declared with too many generic arguments).
For instance, here is a reduced ("reduced" in the sense that it does not trigger additional errors) version of the example given in rust-lang#83893:
```rust
#![feature(lang_items,no_core)]
#![no_core]
#![crate_type="lib"]
#[lang = "sized"]
trait MySized {}
#[lang = "add"]
trait MyAdd<'a, T> {}
fn ice() {
let r = 5;
let a = 6;
r + a
}
```
On current nightly, this immediately causes an ICE without any warnings or errors emitted. With the changes in this PR, however, I get no ICE and two errors:
```
error[E0718]: `add` language item must be applied to a trait with 1 generic argument
--> pr-ex.rs:8:1
|
8 | #[lang = "add"]
| ^^^^^^^^^^^^^^^
9 | trait MyAdd<'a, T> {}
| ------- this trait has 2 generic arguments, not 1
error[E0369]: cannot add `{integer}` to `{integer}`
--> pr-ex.rs:14:7
|
14 | r + a
| - ^ - {integer}
| |
| {integer}
error: aborting due to 2 previous errors
Some errors have detailed explanations: E0369, E0718.
For more information about an error, try `rustc --explain E0369`.
```
@GuillaumeGomezGuillaumeGomez mentioned this pull request May 18, 2021
Closed
@GuillaumeGomezGuillaumeGomez mentioned this pull request May 18, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2021
@bors
Auto merge ofrust-lang#85437- GuillaumeGomez:rollup-3jcirty, r=Guil…
a5560a6
…laumeGomez
Rollup of 7 pull requests
Successful merges:
- rust-lang#84587 (rustdoc: Make "rust code block is empty" and "could not parse code block" warnings a lint (`INVALID_RUST_CODEBLOCKS`))
- rust-lang#85280 (Toggle-wrap items differently than top-doc.)
- rust-lang#85338 (Implement more Iterator methods on core::iter::Repeat)
- rust-lang#85339 (Report an error if a lang item has the wrong number of generic arguments)
- rust-lang#85369 (Suggest borrowing if a trait implementation is found for &/&mut <type>)
- rust-lang#85393 (Suppress spurious errors inside `async fn`)
- rust-lang#85415 (Clean up remnants of BorrowOfPackedField)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 1bfd987 into rust-lang:masterMay 18, 2021
@rustbotrustbot added this to the 1.54.0 milestone May 18, 2021
@JohnTitorJohnTitor mentioned this pull request Jun 6, 2021
Closed
@asquared31415asquared31415 mentioned this pull request Aug 4, 2021
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@varkorvarkorvarkor left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@varkorvarkor

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

1.54.0

Development

Successfully merging this pull request may close these issues.

ICE: panicked at 'index out of bounds: the len is 1 but the index is 1', compiler/rustc_typeck/src/check/method/mod.rs:318:32

5 participants

@FabianWolff@rust-highfive@varkor@bors@rustbot