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

Fix parse error message for meta items #124778

Merged
merged 1 commit into from
May 10, 2024

Conversation

fmease
Copy link
Member

@fmease fmease commented May 5, 2024

Addresses #122796 (comment), cc [@]Thomasdezeeuw.

For attrs inside of a macro like #[doc(alias = $ident)] or #[cfg(feature = $ident)] where $ident is a macro metavariable of fragment kind ident, we used to say the following when expanded (with $identident):

error: expected unsuffixed literal or identifier, found `ident`
  --> weird.rs:6:19
   |
6  |      #[cfg(feature = $ident)]
   |                      ^^^^^^
...
11 | m!(id);
   | ------ in this macro invocation
   |
   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)

This was incorrect and caused confusion, justifiably so (see #122796).

In this position, we only accept/expect unsuffixed literals which consist of numeric & string literals as well as the boolean literals / the keywords / the reserved identifiers false & true but not arbitrary identifiers.

Furthermore, we used to suggest garbage when encountering unexpected non-identifier tokens:

error: expected unsuffixed literal, found `-`
  --> weird.rs:16:17
   |
16 | #[cfg(feature = -1)]
   |                 ^
   |
help: surround the identifier with quotation marks to parse it as a string
   |
16 | #[cfg(feature =" "-1)]
   |                + +

Now we no longer do.

@rustbot
Copy link
Collaborator

rustbot commented May 5, 2024

r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 5, 2024
@@ -1,17 +1,25 @@
//@ compile-flags: -Zdeduplicate-diagnostics=yes
//@ run-rustfix
Copy link
Member Author

@fmease fmease May 5, 2024

Choose a reason for hiding this comment

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

I've removed the rustfix directive since I've added a negative test case and didn't want to add revisions (do they work in combination with rustfix?) or a separate test file.

@fmease fmease force-pushed the fix-diag-msg-parse-meta-item branch from f1ce7e9 to 89736c2 Compare May 6, 2024 00:17
@rust-log-analyzer

This comment has been minimized.

@fmease fmease force-pushed the fix-diag-msg-parse-meta-item branch from 89736c2 to 7f0d04b Compare May 6, 2024 00:40
@Thomasdezeeuw

This comment was marked as off-topic.

@fmease

This comment was marked as off-topic.

@lcnr
Copy link
Contributor

lcnr commented May 6, 2024

r? compiler

@rustbot rustbot assigned oli-obk and unassigned lcnr May 6, 2024
@lcnr
Copy link
Contributor

lcnr commented May 6, 2024

r? compiler

@rustbot rustbot assigned nnethercote and unassigned oli-obk May 6, 2024
parse_invalid_meta_item_unquoted_ident = expected unsuffixed literal, found `{$token}`
.suggestion = surround the identifier with quotation marks to parse it as a string
parse_invalid_meta_item = expected unsuffixed literal, found `{$token}`
.quote_ident_sugg = surround the identifier with quotation marks to make it parse as a string
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.quote_ident_sugg = surround the identifier with quotation marks to make it parse as a string
.quote_ident_sugg = surround the identifier with quotation marks to make it into a string

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion, the current wording is roundabout.

@nnethercote
Copy link
Contributor

r=me with the suggestion considered. Thanks!

@fmease fmease force-pushed the fix-diag-msg-parse-meta-item branch from 7f0d04b to de57ef7 Compare May 10, 2024 07:12
@fmease fmease force-pushed the fix-diag-msg-parse-meta-item branch from de57ef7 to 0ad3c5d Compare May 10, 2024 07:16
@fmease
Copy link
Member Author

fmease commented May 10, 2024

@bors r=nnethercote rollup

@bors
Copy link
Contributor

bors commented May 10, 2024

📌 Commit 0ad3c5d has been approved by nnethercote

It is now in the queue for this repository.

@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 May 10, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 10, 2024
…m, r=nnethercote

Fix parse error message for meta items

Addresses rust-lang#122796 (comment), cc [`@]Thomasdezeeuw.`

For attrs inside of a macro like `#[doc(alias = $ident)]` or `#[cfg(feature = $ident)]` where `$ident` is a macro metavariable of fragment kind `ident`, we used to say the following when expanded (with `$ident` ⟼ `ident`):

```
error: expected unsuffixed literal or identifier, found `ident`
  --> weird.rs:6:19
   |
6  |      #[cfg(feature = $ident)]
   |                      ^^^^^^
...
11 | m!(id);
   | ------ in this macro invocation
   |
   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
```

This was incorrect and caused confusion, justifiably so (see rust-lang#122796).

In this position, we only accept/expect *unsuffixed literals* which consist of numeric & string literals as well as the boolean literals / the keywords / the reserved identifiers `false` & `true` **but not** arbitrary identifiers.

Furthermore, we used to suggest garbage when encountering unexpected non-identifier tokens:

```
error: expected unsuffixed literal, found `-`
  --> weird.rs:16:17
   |
16 | #[cfg(feature = -1)]
   |                 ^
   |
help: surround the identifier with quotation marks to parse it as a string
   |
16 | #[cfg(feature =" "-1)]
   |                + +
```

Now we no longer do.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2024
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#124615 (coverage: Further simplify extraction of mapping info from MIR)
 - rust-lang#124778 (Fix parse error message for meta items)
 - rust-lang#124807 (Migrate `run-make/rustdoc-io-error` to `rmake.rs`)
 - rust-lang#124957 (Make `Ty::builtin_deref` just return a `Ty`)

Failed merges:

 - rust-lang#124888 (Migrate `run-make/rustdoc-output-path` to rmake)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#124615 (coverage: Further simplify extraction of mapping info from MIR)
 - rust-lang#124778 (Fix parse error message for meta items)
 - rust-lang#124797 (Refactor float `Primitive`s to a separate `Float` type)
 - rust-lang#124888 (Migrate `run-make/rustdoc-output-path` to rmake)
 - rust-lang#124957 (Make `Ty::builtin_deref` just return a `Ty`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f605174 into rust-lang:master May 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2024
Rollup merge of rust-lang#124778 - fmease:fix-diag-msg-parse-meta-item, r=nnethercote

Fix parse error message for meta items

Addresses rust-lang#122796 (comment), cc [``@]Thomasdezeeuw.``

For attrs inside of a macro like `#[doc(alias = $ident)]` or `#[cfg(feature = $ident)]` where `$ident` is a macro metavariable of fragment kind `ident`, we used to say the following when expanded (with `$ident` ⟼ `ident`):

```
error: expected unsuffixed literal or identifier, found `ident`
  --> weird.rs:6:19
   |
6  |      #[cfg(feature = $ident)]
   |                      ^^^^^^
...
11 | m!(id);
   | ------ in this macro invocation
   |
   = note: this error originates in the macro `m` (in Nightly builds, run with -Z macro-backtrace for more info)
```

This was incorrect and caused confusion, justifiably so (see rust-lang#122796).

In this position, we only accept/expect *unsuffixed literals* which consist of numeric & string literals as well as the boolean literals / the keywords / the reserved identifiers `false` & `true` **but not** arbitrary identifiers.

Furthermore, we used to suggest garbage when encountering unexpected non-identifier tokens:

```
error: expected unsuffixed literal, found `-`
  --> weird.rs:16:17
   |
16 | #[cfg(feature = -1)]
   |                 ^
   |
help: surround the identifier with quotation marks to parse it as a string
   |
16 | #[cfg(feature =" "-1)]
   |                + +
```

Now we no longer do.
@fmease fmease deleted the fix-diag-msg-parse-meta-item branch May 10, 2024 18:11
GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this pull request Jul 10, 2024
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#124615 (coverage: Further simplify extraction of mapping info from MIR)
 - rust-lang#124778 (Fix parse error message for meta items)
 - rust-lang#124797 (Refactor float `Primitive`s to a separate `Float` type)
 - rust-lang#124888 (Migrate `run-make/rustdoc-output-path` to rmake)
 - rust-lang#124957 (Make `Ty::builtin_deref` just return a `Ty`)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants