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

Give a better error message for duplicate built-in macros #76143

Merged
merged 1 commit into from
Sep 2, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Aug 31, 2020

Minor follow-up to #75176 giving a better error message for duplicate builtin macros. This would have made it a little easier to debug.

r? @petrochenkov

@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Name/path resolution done by `rustc_resolve` specifically A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) labels Aug 31, 2020
@rust-highfive
Copy link
Collaborator

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 31, 2020
@jyn514 jyn514 force-pushed the duplicate-builtin-macros branch from c7dad7a to 245ad9f Compare August 31, 2020 04:32
compiler/rustc_resolve/src/macros.rs Outdated Show resolved Hide resolved
let msg = "attempted to define built-in macro more than once";
self.session
.struct_span_err_with_code(item.span, msg, error_code!(E0773))
.span_note(span, "previously defined here")
Copy link
Contributor

Choose a reason for hiding this comment

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

In practice the current span and the previous span are going to point to the same place though, if we are talking about issues like #75176.

Copy link
Member Author

Choose a reason for hiding this comment

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

For #75176, it would have been the same, yes. However that in itself would have been helpful debugging the issue: it shows that the same crate is being loaded twice. And for problems that are less obscure this will be very helpful since it points straight to the other location that conflicts.

compiler/rustc_resolve/src/macros.rs Outdated Show resolved Hide resolved
@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2020
@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 1, 2020
@petrochenkov
Copy link
Contributor

r=me with #76143 (comment) addressed and commits squashed.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 1, 2020
Previously, this would say no such macro existed, but this was
misleading, since the macro _did_ exist, it was just already seen.

- Say where the macro was previously defined
- Add long-form error message
@jyn514 jyn514 force-pushed the duplicate-builtin-macros branch from 65f25c2 to be2947d Compare September 1, 2020 12:34
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 1, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 1, 2020

@bors r=petrochenkov

@bors
Copy link
Contributor

bors commented Sep 1, 2020

📌 Commit be2947d has been approved by petrochenkov

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 1, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2020
Rollup of 14 pull requests

Successful merges:

 - rust-lang#74880 (Add trailing comma support to matches macro)
 - rust-lang#76074 (Add new `-Z dump-mir-spanview` option)
 - rust-lang#76088 (Add more examples to lexicographic cmp on Iterators.)
 - rust-lang#76099 (Add info about `!` and `impl Trait`)
 - rust-lang#76126 (Use "Fira Sans" for crate list font)
 - rust-lang#76132 (Factor out StmtKind::MacCall fields into `MacCallStmt` struct)
 - rust-lang#76143 (Give a better error message for duplicate built-in macros)
 - rust-lang#76158 (Stabilise link-self-contained option)
 - rust-lang#76201 (Move to intra-doc links for library/core/src/panic.rs)
 - rust-lang#76206 (Make all methods of `std::net::Ipv6Addr` const)
 - rust-lang#76207 (# Move to intra-doc links for library/core/src/clone.rs)
 - rust-lang#76212 (Document lint missing_doc_code_examples is nightly-only)
 - rust-lang#76218 (lexer: Tiny improvement to shebang detection)
 - rust-lang#76221 (Clean up header in `iter` docs for `for` loops)

Failed merges:

r? @ghost
@bors bors merged commit b01d0b1 into rust-lang:master Sep 2, 2020
@jyn514 jyn514 deleted the duplicate-builtin-macros branch September 2, 2020 03:21
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 A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-resolve Area: Name/path resolution done by `rustc_resolve` specifically 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.

6 participants