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

Highlight the const fn if error happened because of a bound on the impl block #88907

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Sep 13, 2021

Currently, for the following code, the compiler produces the errors like the
following:

struct Type<T>(T);

impl<T: Clone> Type<T> {
    const fn f() {}
}
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
  |
  = note: see issue #57563 <https://github.com/rust-lang/rust/issues/57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable

This can be confusing (especially to newcomers) since the error mentions "const fn parameters", but highlights only the impl.

This PR adds function highlighting, changing the error to the following:

error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
4 |     pub const fn f() {}
  |     ---------------- function declared as const here
  |
  = note: see issue #57563 <https://github.com/rust-lang/rust/issues/57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable

I've originally wanted to point directly to const token, but couldn't find a way to get it's span. It seems like this span is lost during the AST -> HIR lowering.

Also, since the errors for object casts in const fns (&T -> &dyn Trait) seem to trigger the same error, this PR accidentally changes these errors too. Not sure if it's desired or how to fix this.

P.S. it's my first time contributing to diagnostics, so feedback is very appreciated!


r? @estebank

@rustbot label: +A-diagnostics

…the impl block

Currently, for the following code, the compiler produces the errors like the
following error:
```rust
struct Type<T>

impl<T: Clone> Type<T> {
	fn const f() {}
}
```
```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```

This can be confusing (especially to newcomers) since the error mentions
"const fn parameters", but highlights only the impl.

This commits adds function highlighting, changing the error to the following:

```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
4 |     pub const fn f() {}
  |     ---------------- function declared as const here
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```
@rustbot rustbot added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 13, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 13, 2021
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Also, since the errors for object casts in const fns (&T -> &dyn Trait) seem to trigger the same error, this PR accidentally changes these errors too. Not sure if it's desired or how to fix this.

Those should probably get their own errors instead of these, as they are today they are a bit confusing, but I think your changes are worth it.

@@ -599,12 +599,21 @@ pub mod ty {
}

fn build_error(&self, ccx: &ConstCx<'_, 'tcx>, span: Span) -> DiagnosticBuilder<'tcx> {
feature_err(
let mut builder = feature_err(
Copy link
Contributor

Choose a reason for hiding this comment

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

The common naming convention for these is usually err, or less commonly, diag.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 14, 2021

📌 Commit 6ec7255 has been approved by estebank

@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 Sep 14, 2021
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 14, 2021
…_a_bound_in_impl_block_error, r=estebank

Highlight the `const fn` if error happened because of a bound on the impl block

Currently, for the following code, the compiler produces the errors like the
following:
```rust
struct Type<T>(T);

impl<T: Clone> Type<T> {
    const fn f() {}
}
```
```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```

This can be confusing (especially to newcomers) since the error mentions "const fn parameters", but highlights only the impl.

This PR adds function highlighting, changing the error to the following:

```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
4 |     pub const fn f() {}
  |     ---------------- function declared as const here
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```

---

I've originally wanted to point directly to `const` token, but couldn't find a way to get it's span. It seems like this span is lost during the AST -> HIR lowering.

Also, since the errors for object casts in `const fn`s (`&T` -> `&dyn Trait`) seem to trigger the same error, this PR accidentally changes these errors too. Not sure if it's desired or how to fix this.

P.S. it's my first time contributing to diagnostics, so feedback is very appreciated!

---

r? `@estebank`

`@rustbot` label: +A-diagnostics
Manishearth added a commit to Manishearth/rust that referenced this pull request Sep 15, 2021
…_a_bound_in_impl_block_error, r=estebank

Highlight the `const fn` if error happened because of a bound on the impl block

Currently, for the following code, the compiler produces the errors like the
following:
```rust
struct Type<T>(T);

impl<T: Clone> Type<T> {
    const fn f() {}
}
```
```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```

This can be confusing (especially to newcomers) since the error mentions "const fn parameters", but highlights only the impl.

This PR adds function highlighting, changing the error to the following:

```text
error[E0658]: trait bounds other than `Sized` on const fn parameters are unstable
 --> ./test.rs:3:6
  |
3 | impl<T: Clone> Type<T> {
  |      ^
4 |     pub const fn f() {}
  |     ---------------- function declared as const here
  |
  = note: see issue rust-lang#57563 <rust-lang#57563> for more information
  = help: add `#![feature(const_fn_trait_bound)]` to the crate attributes to enable
```

---

I've originally wanted to point directly to `const` token, but couldn't find a way to get it's span. It seems like this span is lost during the AST -> HIR lowering.

Also, since the errors for object casts in `const fn`s (`&T` -> `&dyn Trait`) seem to trigger the same error, this PR accidentally changes these errors too. Not sure if it's desired or how to fix this.

P.S. it's my first time contributing to diagnostics, so feedback is very appreciated!

---

r? ``@estebank``

``@rustbot`` label: +A-diagnostics
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 16, 2021
…arth

Rollup of 8 pull requests

Successful merges:

 - rust-lang#87320 (Introduce -Z remap-cwd-prefix switch)
 - rust-lang#88690 (Accept `m!{ .. }.method()` and `m!{ .. }?` statements. )
 - rust-lang#88775 (Revert anon union parsing)
 - rust-lang#88841 (feat(rustc_typeck): suggest removing bad parens in `(recv.method)()`)
 - rust-lang#88907 (Highlight the `const fn` if error happened because of a bound on the impl block)
 - rust-lang#88915 (`Wrapping<T>` has the same layout and ABI as `T`)
 - rust-lang#88933 (Remove implementation of `min_align_of` intrinsic)
 - rust-lang#88951 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e1acdf5 into rust-lang:master Sep 16, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 16, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Sep 19, 2021
…_trait_in_const_fn, r=estebank

Add a separate error for `dyn Trait` in `const fn`

Previously "trait bounds other than `Sized` on const fn parameters are unstable" error was used for both trait bounds (`<T: Trait>`) and trait objects (`dyn Trait`). This was pretty confusing.

This PR adds a separate error for trait objects: "trait objects in const fn are unstable". The error for trait bounds is otherwise intact.

This is follow up to rust-lang#88907

r? `@estebank`

`@rustbot` label: +A-diagnostics
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Sep 19, 2021
…_trait_in_const_fn, r=estebank

Add a separate error for `dyn Trait` in `const fn`

Previously "trait bounds other than `Sized` on const fn parameters are unstable" error was used for both trait bounds (`<T: Trait>`) and trait objects (`dyn Trait`). This was pretty confusing.

This PR adds a separate error for trait objects: "trait objects in const fn are unstable". The error for trait bounds is otherwise intact.

This is follow up to rust-lang#88907

r? ``@estebank``

``@rustbot`` label: +A-diagnostics
@WaffleLapkin WaffleLapkin deleted the targeted_const_fn_with_a_bound_in_impl_block_error branch September 19, 2021 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants