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

Add a label to point to the lacking macro name definition #127557

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

linyihai
Copy link
Contributor

@linyihai linyihai commented Jul 10, 2024

Fixes #126694, but adopts the suggestion from #118295

 --> src/main.rs:1:14
  |
1 | macro_rules! {
  |            ^ put a macro name here

@rustbot
Copy link
Collaborator

rustbot commented Jul 10, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
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 Jul 10, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

LGTM, but one change needed. Afterwards I can approve.

@@ -1439,7 +1439,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
);

if macro_kind == MacroKind::Bang && ident.name == sym::macro_rules {
err.subdiagnostic(MaybeMissingMacroRulesName { span: ident.span });
let primary_span = ident.span;
let label_span = primary_span.with_hi(primary_span.hi() + BytePos(1)).shrink_to_hi();
Copy link
Member

Choose a reason for hiding this comment

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

Manual manipulation of spans can cause issues in situations where we recover in the lexer. I don't think this diagnostic would look a lot worse if we just used primary_span.shrink_to_hi().

Alternatively, find a more "pure" way to express this span (or a similar one) without using bytepos.

Copy link
Contributor Author

@linyihai linyihai Jul 26, 2024

Choose a reason for hiding this comment

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

if we just used primary_span.shrink_to_hi()

This is also looks good to me. And it is more consistent between macro_rules! {} and macro_rules!{}, which will always point to the !

Alternatively, find a more "pure" way to express this span (or a similar one) without using bytepos.

I currently have few time to try this.

@compiler-errors
Copy link
Member

@rustbot author

@rustbot rustbot 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 Jul 25, 2024
@linyihai
Copy link
Contributor Author

@rustbot review

@rustbot rustbot 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 Jul 26, 2024
@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 26, 2024

📌 Commit 2fca4ea has been approved by compiler-errors

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 Jul 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#126575 (Make it crystal clear what lint `type_alias_bounds` actually signifies)
 - rust-lang#127017 (Extend rules of dead code analysis for impls for adts to impls for types refer to adts)
 - rust-lang#127523 (Migrate `dump-ice-to-disk` and `panic-abort-eh_frame` `run-make` tests to rmake)
 - rust-lang#127557 (Add a label to point to the lacking macro name definition)
 - rust-lang#127989 (Migrate `interdependent-c-libraries`, `compiler-rt-works-on-mingw` and `incr-foreign-head-span` `run-make` tests to rmake)
 - rust-lang#128099 (migrate tests/run-make/extern-flag-disambiguates to rmake)
 - rust-lang#128170 (Make Clone::clone a lang item)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c5788d6 into rust-lang:master Jul 26, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Jul 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 26, 2024
Rollup merge of rust-lang#127557 - linyihai:issue-126694, r=compiler-errors

Add a label to point to the lacking macro name definition

Fixes rust-lang#126694, but adopts the suggestion from rust-lang#118295

```
 --> src/main.rs:1:14
  |
1 | macro_rules! {
  |            ^ put a macro name here
```
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.

Confusing error when name for macro definition (macro_rules!) is missing
4 participants