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 diagnostic for calling a function with the same name with unresolved Macro #103140

Merged
merged 2 commits into from
Oct 24, 2022

Conversation

chenyukang
Copy link
Member

Fixes #103112

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 17, 2022
@rust-highfive
Copy link
Collaborator

r? @fee1-dead

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2022
let (span, label) = if let PathResult::Failed { span, label, .. } = path_res {
// try to suggest if it's not a macro, maybe a function
if let PathResult::NonModule(partial_res) = self.resolve_path(
Copy link
Member

Choose a reason for hiding this comment

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

I think you can use maybe_resolve_path to simplify -- don't need to pass a Finalize, for example.

None,
) && partial_res.unresolved_segments() == 0 {
let sm = self.session.source_map();
let span = sm.span_extend_while(span, |c| c == '!').unwrap_or(span);
Copy link
Member

Choose a reason for hiding this comment

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

Can you make the suggestion just remove the !, and not replace macro! with macro? I think you can use something like SourceMap::next_point

Copy link
Member Author

@chenyukang chenyukang Oct 18, 2022

Choose a reason for hiding this comment

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

I have a question for the next_point:

For the debug code:

 let exclamation_span = sm.next_point(span);
 debug!("debug-now span: {:?}", span);
 debug!("debug-now exclamation_span: {:?}", exclamation_span);
 debug!("debug-now exclamation_span source: {:?}", sm.span_to_snippet(exclamation_span));
 debug!("debug-now exclamation_span next : {:?}", sm.next_point(exclamation_span));

Printed out this log:

DEBUG rustc_resolve::macros debug-now span: src/test/ui/suggestions/issue-103112.rs:2:19: 2:24 (#0)
DEBUG rustc_resolve::macros debug-now exclamation_span: src/test/ui/suggestions/issue-103112.rs:2:24: 2:24 (#0)
DEBUG rustc_resolve::macros debug-now exclamation_span source: Ok("")
DEBUG rustc_resolve::macros debug-now exclamation_span next : src/test/ui/suggestions/issue-103112.rs:2:24: 2:25 (#0)

Why span (with lo and hi 2:19: 2:24) 's next_point is not (2:25 2:25), it's (2:24 2:24) instead? seems counterintuitive.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a bug at here?

let end_of_next_point = BytePos(cmp::max(sp.lo().0 + 1, end_of_next_point));

-        let end_of_next_point = BytePos(cmp::max(sp.lo().0 + 1, end_of_next_point));
+        let end_of_next_point = BytePos(cmp::max(sp.hi().0 + 1, end_of_next_point));

Copy link
Member Author

@chenyukang chenyukang Oct 18, 2022

Choose a reason for hiding this comment

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

Because of this bug, we always want to call two times of next_point

For example:

let semi_span = this.sess.source_map().next_point(span);

sp = sm.next_point(sp);

@estebank how do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Add another PR to fix it: #103185

Copy link
Contributor

Choose a reason for hiding this comment

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

@chenyukang thanks for fixing the bug with next_point!

@chenyukang
Copy link
Member Author

chenyukang commented Oct 18, 2022

Will merge after PR: #103185

@rust-log-analyzer

This comment has been minimized.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 18, 2022
…int, r=davidtwco

Fix the bug of next_point in source_map

There is a bug in `next_point`, the new span won't move to next position when be called in the first time.

For this reason, our current code is working like this:
1. When we really want to move to the next position, we called two times of `next_point`
2. Some code which use `next_point` actually done the same thing with `shrink_to_hi`

This fix make sure when `next_point` is called, span will move with the width at least 1, and also work correctly in the scenario of multiple bytes.

Ref: rust-lang#103140 (comment)

r? `@davidtwco`
@estebank
Copy link
Contributor

r=me after #103185 lands and this is rebased

@fee1-dead fee1-dead assigned estebank and unassigned fee1-dead Oct 20, 2022
@fee1-dead fee1-dead 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 Oct 20, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 20, 2022
…t, r=davidtwco

Fix the bug of next_point in source_map

There is a bug in `next_point`, the new span won't move to next position when be called in the first time.

For this reason, our current code is working like this:
1. When we really want to move to the next position, we called two times of `next_point`
2. Some code which use `next_point` actually done the same thing with `shrink_to_hi`

This fix make sure when `next_point` is called, span will move with the width at least 1, and also work correctly in the scenario of multiple bytes.

Ref: rust-lang#103140 (comment)

r? `@davidtwco`
@rustbot rustbot added the A-testsuite Area: The testsuite used to check the correctness of rustc label Oct 20, 2022
@chenyukang
Copy link
Member Author

r? @estebank

RalfJung pushed a commit to RalfJung/miri that referenced this pull request Oct 21, 2022
…dtwco

Fix the bug of next_point in source_map

There is a bug in `next_point`, the new span won't move to next position when be called in the first time.

For this reason, our current code is working like this:
1. When we really want to move to the next position, we called two times of `next_point`
2. Some code which use `next_point` actually done the same thing with `shrink_to_hi`

This fix make sure when `next_point` is called, span will move with the width at least 1, and also work correctly in the scenario of multiple bytes.

Ref: rust-lang/rust#103140 (comment)

r? `@davidtwco`
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 22, 2022

📌 Commit f90bf50 has been approved by estebank

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 22, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 23, 2022
…tebank

Add diagnostic for calling a function with the same name with unresolved Macro

Fixes rust-lang#103112
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2022
Rollup of 11 pull requests

Successful merges:

 - rust-lang#100462 (Clarify `array::from_fn` documentation)
 - rust-lang#101644 (Document surprising and dangerous fs::Permissions behaviour on Unix)
 - rust-lang#103005 (kmc-solid: Handle errors returned by `SOLID_FS_ReadDir`)
 - rust-lang#103140 (Add diagnostic for calling a function with the same name with unresolved Macro)
 - rust-lang#103254 (rustdoc: do not filter out cross-crate `Self: Sized` bounds)
 - rust-lang#103347 (bootstrap: also create rustc-src component in sysroot)
 - rust-lang#103402 (Fix wrapped valid-range handling in ty_find_init_error)
 - rust-lang#103414 (Pretty print lifetimes captured by RPIT)
 - rust-lang#103424 (rustdoc: remove no-op CSS `.code-header { border-bottom: none }`)
 - rust-lang#103434 (Use functions for jump-to-def-background rustdoc GUI test)
 - rust-lang#103447 (`MaybeUninit`: use `assume_init_drop()` in the partially initialized array example)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3df030d into rust-lang:master Oct 24, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 24, 2022
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
…dtwco

Fix the bug of next_point in source_map

There is a bug in `next_point`, the new span won't move to next position when be called in the first time.

For this reason, our current code is working like this:
1. When we really want to move to the next position, we called two times of `next_point`
2. Some code which use `next_point` actually done the same thing with `shrink_to_hi`

This fix make sure when `next_point` is called, span will move with the width at least 1, and also work correctly in the scenario of multiple bytes.

Ref: rust-lang/rust#103140 (comment)

r? `@davidtwco`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…dtwco

Fix the bug of next_point in source_map

There is a bug in `next_point`, the new span won't move to next position when be called in the first time.

For this reason, our current code is working like this:
1. When we really want to move to the next position, we called two times of `next_point`
2. Some code which use `next_point` actually done the same thing with `shrink_to_hi`

This fix make sure when `next_point` is called, span will move with the width at least 1, and also work correctly in the scenario of multiple bytes.

Ref: rust-lang/rust#103140 (comment)

r? `@davidtwco`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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.

When a macro isn't found, suggest calling a function with the same name
8 participants