Continue refactoring macro expansion and resolution #62476
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @eddyb @davidtwco |
This was referenced Jul 7, 2019
pnkfelix
reviewed
Jul 9, 2019
There was a problem hiding this comment.
I found this note in the commit message (cec77d2ba093d108eadc95dfd2ccd0f84991cd9a) interesting:
it was called before all the data required for resolution is available, so it could work incorrectly in some corner cases (like user-defined derives name
CopyorEq)
Can you make some regression tests illustrating these cases that used to fail, and add them either in a later commit in this PR, or in a follow-up PR?
Added the test. |
petrochenkov
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #62555) made this pull request unmergeable. Please resolve the merge conflicts. |
matthewjasper
approved these changes
Jul 10, 2019
|
|
||
| macro mac() { | ||
| mod m { | ||
| use std::mem; // OK (extern prelude) |
There was a problem hiding this comment.
I guess this means that the following is no longer accepted:
// In the crate root:
macro a() {
extern crate core as my_core;
mod v {
use my_core;
}
}
a!();Maybe this deserves a test?
There was a problem hiding this comment.
Yes, this no longer works.
Prelude names exist at the whole crate level and are supposed to look the same from every its point, macro expanded or not.
This means no one can see prelude entries introduced by opaque macros.
There was a problem hiding this comment.
I've added a test.
It's internal to resolve and always results in `Res::Err` outside of resolve. Instead put `DefKind::Fn`s themselves into the macro namespace, it's ok. Proc macro stubs are items placed into macro namespase for functions that define proc macros. rust-lang#52383 The rustdoc test is changed because the old test didn't actually reproduce the ICE it was supposed to reproduce.
So it can be eventually used in `ExpnInfo`
More consistent with other naming: ExpnFormat -> ExpnKind ExpnKind::name -> ExpnKind::descr DesugaringKind::name -> DesugaringKind::descr Shorter, no tautology: CompilerDesugaring -> Desugaring CompilerDesugaringKind -> DesugaringKind
The expansions were created to allow unstable things inside `#[test_case/test/bench]`, but that's not a proper way to do that. Put the required `allow_internal_unstable`s into the macros' properties instead.
It was used to choose whether to apply derive markers like `#[rustc_copy_clone_marker]` or not, but it was called before all the data required for resolution is available, so it could work incorrectly in some corner cases (like user-defined derives name `Copy` or `Eq`). Delay the decision about markers until the proper resolution results are available instead.
…osticSpanMacroExpansion We have to deal with dummy spans anyway Remove def-site span from expander interfaces. It's not used by the expansion infra, only by specific expanders, which can keep it themselves if they want it.
Orthogonality and reuse are good.
Avoid the tricky scheme with callbacks and keep the invocation parent data where it logically belongs - in `Definitions`. This also allows to create `InvocationData` entries in resolve when the data is actually ready, and remove cells and "uninitialized" variants from it.
Create real working and registered (even if dummy) `SyntaxExtension`s for them. This improves error recovery and allows to avoid all special cases for proc macro stubs (except for the error on use, of course). The introduced dummy `SyntaxExtension`s can be used for any other inappropriately resolved macros as well.
…ate errors It either returns the indeterminacy error, or valid (but perhaps dummy) `SyntaxExtension`. With this change enum `Determinacy` is no longer used in libsyntax and can be moved to resolve. The regressions in diagnosics are fixed in the next commits.
This way we are processing all of them in a single point, rather than separately for each syntax extension kind. Also, the standard expected/found wording is used.
This is going to be used when built-in macros are defined through libcore and made available to other crates through standard library prelude
…rting parts Also move macro stability checking closer to other checks performed on obtained resolutions. Tighten the stability spans as well, it is an error to *refer* to and unstable entity in any way, not only "call" it.
Ok, it's hard to explain what happens, but identifier's hygienic contexts need to be "adjusted" to modules/scopes before they are resolved in them.
To be resolved in all kinds on preludes the identifier needs to be adjusted to the root expansion (aka "no expansion").
Previously this was done for the `macro m() { ::my_crate::foo }` case, but forgotten for all other cases.
…rkers Create a fresh expansion for them instead - this is the usual way to allow unstable features for generated/desugared code. Fixes rust-lang#52363
Creating a fresh expansion and immediately generating a span from it is the most common scenario. Also avoid allocating `allow_internal_unstable` lists for derive markers repeatedly. And rename `ExpnInfo::with_unstable` to `ExpnInfo::allow_unstable`, seems to be a better fitting name.
Use variant names rather than descriptions for identifying desugarings in `#[rustc_on_unimplemented]`. Both are highly unstable, but variant name is at least a single identifier.
The root expansion was missing one. Expansions created for "derive containers" (see one of the next commits for the description) also didn't get expansion info.
Remove a bunch of `Option`s that assumed that dummy fragment creation could fail. The test output changed due to not performing the expansion in `fn expand_invoc` in case of the recursion limit hit.
It's more convenient to have all this highly related stuff together on one screen (for future refactorings). The `expand_invoc` function is compact enough now, after all the previous refactorings.
…iant
`InvocationKind::Attr { attr: None, .. }` meaning something entirely different from a regular attribute was confusing as hell.
Add a test for the issue resolved by removing `resolve_macro_path` Add a test making sure that extern prelude entries introduced from an opaque macro are not visible anywhere, even it that macro Fix test output after rebase
|
@bors r=matthewjasper |
|
📌 Commit e86e5cb has been approved by |
bors
added
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Jul 11, 2019
Continue refactoring macro expansion and resolution This PR continues the work started in rust-lang#62042. It contains a set of more or less related refactorings with the general goal of making things simpler and more orthogonal. Along the way most of the issues uncovered in rust-lang#62086 are fixed. The PR is better read in per-commit fashion with whitespace changes ignored. I tried to leave some more detailed commit messages describing the motivation behind the individual changes. Fixes rust-lang#44692 Fixes rust-lang#52363 Unblocks rust-lang#62086 r? @matthewjasper
bors
added a commit
that referenced
this pull request
Jul 11, 2019
Rollup of 7 pull requests Successful merges: - #61665 (core: check for pointer equality when comparing Eq slices) - #61923 (Prerequisites from dep graph refactoring #2) - #62270 (Move async-await tests from run-pass to ui) - #62425 (filedesc: don't use ioctl(FIOCLEX) on Linux) - #62476 (Continue refactoring macro expansion and resolution) - #62519 (Regression test for HRTB bug (issue 30786).) - #62557 (Fix typo in libcore/intrinsics.rs) Failed merges: r? @ghost
bors
added a commit
to rust-lang/rust-clippy
that referenced
this pull request
Jul 11, 2019
Rustup `macro expansion and resolution` Rustup rust-lang/rust#62476 changelog: none
bors
added a commit
to rust-lang/rust-clippy
that referenced
this pull request
Jul 12, 2019
Rustup `macro expansion and resolution` Rustup rust-lang/rust#62476 changelog: none
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR continues the work started in #62042.
It contains a set of more or less related refactorings with the general goal of making things simpler and more orthogonal.
Along the way most of the issues uncovered in #62086 are fixed.
The PR is better read in per-commit fashion with whitespace changes ignored.
I tried to leave some more detailed commit messages describing the motivation behind the individual changes.
Fixes #44692
Fixes #52363
Unblocks #62086
r? @matthewjasper