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

Suggest importing macros from the crate root #59784

Merged
merged 6 commits into from Apr 14, 2019

Conversation

Projects
None yet
5 participants
@davidtwco
Copy link
Member

commented Apr 7, 2019

Fixes #59764.

r? @estebank
cc @varkor

Add test with current behaviour.
This commit adds a test demonstrating the current behaviour when a macro
defined in a module with the `#[macro_export]` is imported from the
module rather than the crate root.
@estebank
Copy link
Contributor

left a comment

It seems ok to me, but there's quite a bit of span wrangling here that makes me uneasy (but is sometimes unavoidable). I'll take a closer look at that code later. For now the only complaint I'd have is the separation of the note into 3. Relying on terminal word-wrapping is reasonable in my mind.

Show resolved Hide resolved src/librustc_resolve/error_reporting.rs Outdated

@davidtwco davidtwco force-pushed the davidtwco:issue-59764 branch from 22aabf6 to ad4412f Apr 10, 2019

@davidtwco

This comment has been minimized.

Copy link
Member Author

commented Apr 10, 2019

It seems ok to me, but there's quite a bit of span wrangling here that makes me uneasy (but is sometimes unavoidable).

I only span-wrangled here because I couldn't see another way in this case. If you can think of an alternative, I'll happily take that approach. While I have introduced some more span-wrangling with this PR, I was able to re-use a bunch of the existing span-wrangling from a previous PR, some of the diff is just extracting that existing logic into functions.

Show resolved Hide resolved src/librustc_resolve/error_reporting.rs Outdated
Show resolved Hide resolved src/librustc_resolve/error_reporting.rs
Show resolved Hide resolved src/librustc_resolve/error_reporting.rs Outdated
Show resolved Hide resolved src/librustc_resolve/error_reporting.rs Outdated

davidtwco added some commits Apr 7, 2019

Suggest macro import from crate root.
This commit suggests importing a macro from the root of a crate as the
intent may have been to import a macro from the definition location that
was annotated with `#[macro_export]`.
Handle renamed imports.
This commit extends the suggestion to handle imports that are aliased to
another name.
Handle edge cases.
This commit introduces more dirty span manipulation into the compiler
in order to handle the various edge cases in moving/renaming the macro
import so it is at the root of the import.
Improve robustness of nested check.
This commit removes the assumption that the start of a use statement
will always be on one line with a single space - which was silly in the
first place.
Switch to multipart suggestions.
This commit changes the suggestion so that it is split into multiple
parts in an effort to reduce the impact the applied suggestion could
have on formatting.

@davidtwco davidtwco force-pushed the davidtwco:issue-59764 branch from ad4412f to 5158063 Apr 11, 2019

@estebank

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

r=me once travis finishes

@estebank

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

📌 Commit 5158063 has been approved by estebank

Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019

Rollup merge of rust-lang#59784 - davidtwco:issue-59764, r=estebank
Suggest importing macros from the crate root

Fixes rust-lang#59764.

r? @estebank
cc @varkor

Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019

Rollup merge of rust-lang#59938 - Centril:rollup-u6r5nca, r=Centril
Rollup of 9 pull requests

Successful merges:

 - rust-lang#59655 (Use a proc macro to declare preallocated symbols)
 - rust-lang#59769 (compiletest normalization: preserve non-JSON lines such as ICEs)
 - rust-lang#59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - rust-lang#59784 (Suggest importing macros from the crate root)
 - rust-lang#59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - rust-lang#59856 (update polonius-engine)
 - rust-lang#59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - rust-lang#59890 (Don't generate empty json variables)
 - rust-lang#59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019

Rollup merge of rust-lang#59784 - davidtwco:issue-59764, r=estebank
Suggest importing macros from the crate root

Fixes rust-lang#59764.

r? @estebank
cc @varkor

bors added a commit that referenced this pull request Apr 13, 2019

Auto merge of #59942 - Centril:rollup-6f3dag0, r=Centril
Rollup of 8 pull requests

Successful merges:

 - #59655 (Use a proc macro to declare preallocated symbols)
 - #59769 (compiletest normalization: preserve non-JSON lines such as ICEs)
 - #59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - #59784 (Suggest importing macros from the crate root)
 - #59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - #59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - #59890 (Don't generate empty json variables)
 - #59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost

Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019

Rollup merge of rust-lang#59784 - davidtwco:issue-59764, r=estebank
Suggest importing macros from the crate root

Fixes rust-lang#59764.

r? @estebank
cc @varkor

bors added a commit that referenced this pull request Apr 14, 2019

Auto merge of #59950 - Centril:rollup-hpmr62i, r=Centril
Rollup of 6 pull requests

Successful merges:

 - #59776 (Apply resource-suffix to search-index and source-files scripts as well)
 - #59784 (Suggest importing macros from the crate root)
 - #59812 (Exclude profiler-generated symbols from MSVC __imp_-symbol workaround.)
 - #59874 (Clean up handling of `-Z pgo-gen` commandline option.)
 - #59890 (Don't generate empty json variables)
 - #59911 (Revert "compile crates under test w/ -Zemit-stack-sizes")

Failed merges:

r? @ghost

@bors bors merged commit 5158063 into rust-lang:master Apr 14, 2019

@davidtwco davidtwco deleted the davidtwco:issue-59764 branch Apr 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.