Tweak macro parse errors when reaching EOF during macro call parse #61026

estebank · 2019-05-22T01:12:37Z

Add detail on origin of current parser when reaching EOF, stop saying "found <eof>" and point at the end of macro calls.

Fix #27569.

rust-highfive · 2019-05-22T01:12:48Z

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2019-05-22T01:12:49Z

r? @petrochenkov

src/libsyntax/config.rs

src/libsyntax/ext/tt/macro_parser.rs

src/libsyntax/parse/mod.rs

src/libsyntax/parse/parser.rs

src/test/ui/parser/bad-macro-argument.rs

src/libsyntax/parse/parser.rs

src/libsyntax/config.rs

petrochenkov · 2019-05-23T18:29:03Z

src/libsyntax/ext/derive.rs

@@ -17,8 +17,11 @@ pub fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec<ast::Attribute>) ->
            return true;
        }
        if !attr.is_meta_item_list() {
-            cx.span_err(attr.span,
-                        "attribute must be of the form `#[derive(Trait1, Trait2, ...)]`");


The output was supposed to be identical to the generic attribute input validation infra in feature_gate.rs.
(Some attributes like cfg or derive are very special, and have to be checked before that generic infra works.)

I removed plenty of attribute-specific messages like "missing traits to be derived" in #57321 because the generic messages are good enough and intuitive.

The cfg_attr check should probably follow the same template.
I still hope that the special attributes can be made less special and go through the generic infra one day.

I was considering (but obviously didn't do it in this PR) moving the generic errors to this new format. Is the concern about these being inconsistent or fundamentally against the new output? If it is the former, should I just do that work here for you to see what it'd look like?

Fist, consistency, second, I don't like the new wording.
"Bad attribute" doesn't says that the issue is with the attribute input specifically, and not with name or something, the format suggestion is moved too far into a note.
The status quo looks good to me (that's not surprising, since I wrote it) and I don't understand the motivation behind the change.

If I'd change anything, I'd rather move the "must be of the form #[...]" to a suggestion, since it's indeed an approximate suggesion (IIRC, it will be rendered as a label then and will stay close enough).
For the primary message in that case I'd choose something mentioning input, e.g. "ill-formed attribute input".

I made these changes and it ballooned the size of the PR, I hope that's ok.

FWIW, you can move the EOF staff into a separate PR and consider it r+'d.
I'll be mostly unavailable in the next few days, so this PR may land with a delay.

Kept the EOF stuff here and opened a new one with the rewording at #61140

- Add detail on origin of current parser when reaching EOF and stop saying "found <eof>" and point at the end of macro calls - Handle empty `cfg_attr` attribute - Reword empty `derive` attribute error

estebank · 2019-05-24T19:02:38Z

src/test/ui/on-unimplemented/bad-annotation.stderr

  --> $DIR/bad-annotation.rs:17:1
   |
 LL | #[rustc_on_unimplemented]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+help: the following are the possible correct uses
+   |
+LL | #[rustc_on_unimplemented(/*opt*/ message = "...", /*opt*/ label = "...", /*opt*/ note = "...")]


Right now it's not possible to have more than one suggestion per input type, but now that we suggest we could add it and have a suggestion for each combination (this is probably not best example that would benefit of this).

estebank · 2019-05-24T19:02:55Z

src/test/ui/no_crate_type.stderr

  --> $DIR/no_crate_type.rs:2:1
   |
 LL | #![crate_type]
-   | ^^^^^^^^^^^^^^
+   | ^^^^^^^^^^^^^^ help: must be of the form: `#[crate_type = "bin|lib|..."]`


Would benefit of multiple suggestions of the same type.

estebank · 2019-05-24T19:04:49Z

src/test/ui/malformed/malformed-unwind-1.stderr

  --> $DIR/malformed-unwind-1.rs:3:1
   |
 LL | #[unwind]
-   | ^^^^^^^^^
+   | ^^^^^^^^^ help: must be of the form: `#[unwind(allowed|aborts)]`


Would benefit of multiple suggestions

estebank · 2019-05-24T19:09:37Z

src/test/ui/lint/reasons-erroneous.stderr

  --> $DIR/reasons-erroneous.rs:9:29
   |
 LL | #![warn(bare_trait_objects, reasons = "leaders to no sure land, guides their bearings lost")]
-   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ bad attribute argument


Should we add levenshtein distance enabled suggestions?

estebank · 2019-05-24T19:10:44Z

src/test/ui/lint/reasons-erroneous.stderr

-   |                                                          ^
-   |
-   = help: reason must be a string literal
+   |                                                          ^ reason must be a string literal


Could suggest "0".

estebank · 2019-05-24T19:11:01Z

src/test/ui/lint/reasons-erroneous.stderr

-   |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: reason must be a string literal
+   |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reason must be a string literal


Could suggest removing the b.

estebank · 2019-05-24T19:30:36Z

CC @rust-lang/wg-diagnostics for more eyes

estebank · 2019-05-25T00:04:54Z

@bors r=petrochenkov

as per #61026 (comment), the extraneous changes have been moved to #61140

bors · 2019-05-25T00:04:55Z

📌 Commit ee7593e has been approved by petrochenkov

…enkov Tweak macro parse errors when reaching EOF during macro call parse Add detail on origin of current parser when reaching EOF, stop saying "found `<eof>`" and point at the end of macro calls. Fix rust-lang#27569.

@ghost

Rollup of 13 pull requests Successful merges: - rust-lang#61026 (Tweak macro parse errors when reaching EOF during macro call parse) - rust-lang#61095 (Update cargo) - rust-lang#61096 (tidy: don't short-circuit on license error) - rust-lang#61107 (Fix a couple docs typos) - rust-lang#61110 (Revert edition-guide toolstate override) - rust-lang#61111 (Fixed type-alias-bounds lint doc) - rust-lang#61113 (Deprecate `FnBox`. `Box<dyn FnOnce()>` can be called directly, since 1.35) - rust-lang#61116 (Remove the incorrect warning from README.md) - rust-lang#61118 (Dont ICE on an attempt to use GAT without feature gate) - rust-lang#61121 (improve debug-printing of scalars) - rust-lang#61125 (Updated my mailmap entry) - rust-lang#61134 (Annotate each `reverse_bits` with `#[must_use]`) - rust-lang#61138 (Move async/await tests to their own folder) Failed merges: r? @ghost

@ghost

Rollup of 13 pull requests Successful merges: - #61026 (Tweak macro parse errors when reaching EOF during macro call parse) - #61095 (Update cargo) - #61096 (tidy: don't short-circuit on license error) - #61107 (Fix a couple docs typos) - #61110 (Revert edition-guide toolstate override) - #61111 (Fixed type-alias-bounds lint doc) - #61113 (Deprecate `FnBox`. `Box<dyn FnOnce()>` can be called directly, since 1.35) - #61116 (Remove the incorrect warning from README.md) - #61118 (Dont ICE on an attempt to use GAT without feature gate) - #61121 (improve debug-printing of scalars) - #61125 (Updated my mailmap entry) - #61134 (Annotate each `reverse_bits` with `#[must_use]`) - #61138 (Move async/await tests to their own folder) Failed merges: r? @ghost

Rustup to rust-lang/rust#61026 None

Changes: ```` Rustup to rust-lang#61026 rustup rust-lang#60803 Rustup to rust-lang#59545 Rustup to rust-lang#60965 clippy: bump rustc_tools util version to 0.2 rustc_tools_util: fix typo in docs (readme) rustc_tool_utils: bump version to 0.2.0 update if_chain to 1.0.0 tests: update needless_bool test stderr cargo fmt Rustup to rust-lang#60740 Lifetimes UI test cleanup ````

…lwoerister Reword malformed attribute input diagnostics - Handle empty `cfg_attr` attribute - Reword empty `derive` attribute error - Use consistend error message: "malformed `attrname` attribute input" - Provide suggestions when possible - Move note/help to label/suggestion - Use consistent wording "ill-formed" -> "malformed" - Move diagnostic logic out of parser Split up from rust-lang#61026, where there's prior conversation.

Reword malformed attribute input diagnostics - Handle empty `cfg_attr` attribute - Reword empty `derive` attribute error - Use consistend error message: "malformed `attrname` attribute input" - Provide suggestions when possible - Move note/help to label/suggestion - Use consistent wording "ill-formed" -> "malformed" - Move diagnostic logic out of parser Split up from #61026, where there's prior conversation.

rust-highfive assigned michaelwoerister May 22, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 22, 2019

rust-highfive assigned petrochenkov and unassigned michaelwoerister May 22, 2019

Centril reviewed May 22, 2019

View reviewed changes