Skip to content

Fix parse error message for meta items#124778

Merged
bors merged 1 commit intorust-lang:masterfrom
fmease:fix-diag-msg-parse-meta-item
May 10, 2024
Merged

Fix parse error message for meta items#124778
bors merged 1 commit intorust-lang:masterfrom
fmease:fix-diag-msg-parse-meta-item

Conversation

@fmease
Copy link
Member

@fmease fmease commented May 5, 2024 •
edited
Loading

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 $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 #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
fmease
fmease commented May 5, 2024
View reviewed changes
tests/ui/parser/attribute/attr-unquoted-ident.rs
@@ -1,17 +1,25 @@
//@ compile-flags: -Zdeduplicate-diagnostics=yes
//@ run-rustfix
Copy link
Member Author

@fmease fmease May 5, 2024 •
edited
Loading

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.

Sign in to view
@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.

Sign in to view
@fmease

This comment was marked as off-topic.

Sign in to view
@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
nnethercote
nnethercote reviewed May 10, 2024
View reviewed changes
compiler/rustc_parse/messages.ftl Outdated
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

@nnethercote nnethercote May 10, 2024

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

@fmease fmease May 10, 2024

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

nnethercote commented May 10, 2024

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
Collaborator

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
@matthiaskrgr
Rollup merge of rust-lang#124778 - fmease:fix-diag-msg-parse-meta-ite…
26cf66c 
…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.
@matthiaskrgr matthiaskrgr mentioned this pull request May 10, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2024
@bors
Auto merge of rust-lang#124964 - matthiaskrgr:rollup-cxz1jmi, r=matth…
f21b1f2 
…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
@matthiaskrgr matthiaskrgr mentioned this pull request May 10, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2024
@bors
Auto merge of rust-lang#124972 - matthiaskrgr:rollup-3fablim, r=matth…
6a19a87 
…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
@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
@rust-timer
Unrolled build for rust-lang#124778
6aa985c 
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
@bors
Auto merge of rust-lang#124972 - matthiaskrgr:rollup-3fablim, r=matth…
06f1ba4 
…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

Reviewers

@nnethercote nnethercote nnethercote left review comments

Assignees

@nnethercote nnethercote

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

Milestone

1.80.0

Development

Successfully merging this pull request may close these issues.

8 participants

@fmease @rustbot @rust-log-analyzer @Thomasdezeeuw @lcnr @nnethercote @bors @oli-obk