{"action":"created","issue":{"url":"https://api.github.com/repos/rust-lang/rust/issues/54841","repository_url":"https://api.github.com/repos/rust-lang/rust","labels_url":"https://api.github.com/repos/rust-lang/rust/issues/54841/labels{/name}","comments_url":"https://api.github.com/repos/rust-lang/rust/issues/54841/comments","events_url":"https://api.github.com/repos/rust-lang/rust/issues/54841/events","html_url":"https://github.com/rust-lang/rust/issues/54841","id":367149683,"node_id":"MDU6SXNzdWUzNjcxNDk2ODM=","number":54841,"title":"Incorrect diagnostic span on syntax error in macro invoc inside trait, impl, extern blocks","user":{"login":"abonander","id":3198595,"node_id":"MDQ6VXNlcjMxOTg1OTU=","avatar_url":"https://avatars1.githubusercontent.com/u/3198595?v=4","gravatar_id":"","url":"https://api.github.com/users/abonander","html_url":"https://github.com/abonander","followers_url":"https://api.github.com/users/abonander/followers","following_url":"https://api.github.com/users/abonander/following{/other_user}","gists_url":"https://api.github.com/users/abonander/gists{/gist_id}","starred_url":"https://api.github.com/users/abonander/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abonander/subscriptions","organizations_url":"https://api.github.com/users/abonander/orgs","repos_url":"https://api.github.com/users/abonander/repos","events_url":"https://api.github.com/users/abonander/events{/privacy}","received_events_url":"https://api.github.com/users/abonander/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-10-05T10:22:41Z","updated_at":"2018-10-05T21:48:25Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"A syntax error produced by a macro invocation in a `trait {}` or `impl {}` block will have an incorrect span for its second label:\r\n\r\n[Playground](https://play.rust-lang.org/?gist=7a0fd6c77f665d89f5b40ec949e9b93f&version=stable&mode=debug&edition=2015)\r\n```rust\r\nmacro_rules! m {\r\n    () => {\r\n        let\r\n    }\r\n}\r\n\r\ntrait Foo {\r\n    m!();\r\n}\r\n```\r\n\r\nProduces this error:\r\n```\r\nerror: expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found `let`\r\n --> src/lib.rs:3:9\r\n  |\r\n1 | macro_rules! m {\r\n  | - expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe` here\r\n2 |     () => {\r\n3 |         let\r\n  |         ^^^ unexpected token\r\n...\r\n8 |     m!();\r\n  |     ----- in this macro invocation\r\n```\r\n\r\nThe \"expected one of ... here\" line is pointing to line 1 but it should be elaborating on the \"unexpected token\" label, or just replacing it. The behavior is identical for `impl {}` blocks.\r\n\r\nThis appears to be due to some faulty logic in [`Parser::expect_one_of()`](https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L681) which isn't robust in the face of macro invocations. If it can't get lines for either span from the sourcemap it should assume they're the same (in fact I'm not sure why it emits a separate \"unexpected token\" label).\r\n\r\nDiscovered in #54833 but not introduced by it; when that's merged `extern {}` blocks will show this behavior too (as seen in `issue-54441.stderr` on that PR).\r\n\r\nI'll try to look into this sometime this weekend but I'm also willing to mentor on it, though like I mentioned I'm not sure why the error handling logic in `Parser::expect_one_of()` is so complex to begin with.\r\n\r\ncc @petrochenkov "},"comment":{"url":"https://api.github.com/repos/rust-lang/rust/issues/comments/427508057","html_url":"https://github.com/rust-lang/rust/issues/54841#issuecomment-427508057","issue_url":"https://api.github.com/repos/rust-lang/rust/issues/54841","id":427508057,"node_id":"MDEyOklzc3VlQ29tbWVudDQyNzUwODA1Nw==","user":{"login":"estebank","id":1606434,"node_id":"MDQ6VXNlcjE2MDY0MzQ=","avatar_url":"https://avatars2.githubusercontent.com/u/1606434?v=4","gravatar_id":"","url":"https://api.github.com/users/estebank","html_url":"https://github.com/estebank","followers_url":"https://api.github.com/users/estebank/followers","following_url":"https://api.github.com/users/estebank/following{/other_user}","gists_url":"https://api.github.com/users/estebank/gists{/gist_id}","starred_url":"https://api.github.com/users/estebank/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/estebank/subscriptions","organizations_url":"https://api.github.com/users/estebank/orgs","repos_url":"https://api.github.com/users/estebank/repos","events_url":"https://api.github.com/users/estebank/events{/privacy}","received_events_url":"https://api.github.com/users/estebank/received_events","type":"User","site_admin":false},"created_at":"2018-10-05T21:48:25Z","updated_at":"2018-10-05T21:48:25Z","author_association":"CONTRIBUTOR","body":"This is because when [looking for the prior span in the current context](https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L717-L733) in order to [check wether the prior span is in a different line](https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L760-L774) the code doesn't account for the case where we're in a macro context, making `self.prev_span == DUMMY_SP`, which is followed by the first char in the source code. Because of this, [we can change the following code](https://github.com/rust-lang/rust/blob/master/src/libsyntax/parse/parser.rs#L759-L779):\r\n\r\n```\r\n            match (cm.lookup_line(self.span.lo()), cm.lookup_line(sp.lo())) {\r\n                (Ok(ref a), Ok(ref b)) if a.line == b.line => {\r\n                    // When the spans are in the same line, it means that the only content between\r\n                    // them is whitespace, point at the found token in that case:\r\n                    //\r\n                    // X |     () => { syntax error };\r\n                    //   |                    ^^^^^ expected one of 8 possible tokens here\r\n                    //\r\n                    // instead of having:\r\n                    //\r\n                    // X |     () => { syntax error };\r\n                    //   |                   -^^^^^ unexpected token\r\n                    //   |                   |\r\n                    //   |                   expected one of 8 possible tokens here\r\n                    err.span_label(self.span, label_exp);\r\n                }\r\n                _ => {\r\n                    err.span_label(sp, label_exp);\r\n                    err.span_label(self.span, \"unexpected token\");\r\n                }\r\n            }\r\n```\r\n\r\nto \r\n\r\n```\r\n            match (cm.lookup_line(self.span.lo()), cm.lookup_line(sp.lo())) {\r\n                (Ok(ref a), Ok(ref b)) if a.line == b.line => {\r\n                    // When the spans are in the same line, it means that the only content between\r\n                    // them is whitespace, point at the found token in that case:\r\n                    //\r\n                    // X |     () => { syntax error };\r\n                    //   |                    ^^^^^ expected one of 8 possible tokens here\r\n                    //\r\n                    // instead of having:\r\n                    //\r\n                    // X |     () => { syntax error };\r\n                    //   |                   -^^^^^ unexpected token\r\n                    //   |                   |\r\n                    //   |                   expected one of 8 possible tokens here\r\n                    err.span_label(self.span, label_exp);\r\n                }\r\n                _ if self.prev_span == syntax_pos::DUMMY_SP => {\r\n                    // Account for macro context where the previous span might not be\r\n                    // available to avoid incorrect output (#54841).\r\n                    err.span_label(self.span, \"unexpected token\");\r\n                }\r\n                _ => {\r\n                    err.span_label(sp, label_exp);\r\n                    err.span_label(self.span, \"unexpected token\");\r\n                }\r\n            }\r\n```"}}
{"actor":{"display_login":"estebank"}}