Continue refactoring resolve and hygiene by petrochenkov · Pull Request #63535 · rust-lang/rust

petrochenkov · 2019-08-13T21:27:12Z

The general goal is addressing FIXMEs from the previous PRs.

Merging similar data structures (+ prerequisites for such merging), accounting for the fact that all ExpnIds have associated data in HygieneData now (less Options).

Also, some renaming.
This should be the last renaming session in this area, I think.

r? @matthewjasper

matklad · 2019-08-13T21:47:54Z

src/librustc_resolve/lib.rs

Defeault with parameter looks odd. Perhaps module_scope?

I agree that default isn't great here, but I'm not sure what to call it. Either way, I feel like a comment for this function would be helpful.

Renamed into ParentScope::module and some comments are added.

matklad · 2019-08-13T21:50:23Z

src/librustc_resolve/build_reduced_graph.rs

👍 I find privacy super useful when reading new parts of the compiler

matthewjasper · 2019-08-15T11:47:59Z

r=me with conflicts resolved, with or without the comment being addressed.

… expansion

…::root()` For consistency with `ExpnId::root`. Also introduce a helper `Span::with_root_ctxt` for creating spans with `SyntaxContext::root()` context

`Ident` has had a full span rather than just a `SyntaxContext` for a long time now.

The expansion info is not optional and should always exist

Traces already contain module info without that. It's easy to forget to call `finalize_*` on a module. In particular, macros enum and trait modules weren't finalized. By happy accident macros weren't placed into those modules until now.

The previous approach was brittle - what would happen if `ParentScope` wasn't created by `invoc_parent_scope`? That's exactly the case for various uses of `ParentScope` in diagnostics and in built-in attribute validation.

Remove some unnecessary parameters from functions

It was very similar to `ParentScope` and mostly could be replaced by it.

By allocating its derive paths on the resolver arena.

It was introduced to avoid going through `hygiene_data`, but now it's read only once, when `ParseSess` is created, so going through a lock is ok.

For naming consistency with everything else in this area

petrochenkov · 2019-08-15T18:05:20Z

@bors r=matthewjasper

bors · 2019-08-15T18:05:22Z

📌 Commit c762773 has been approved by matthewjasper

@matthewjasper

Continue refactoring resolve and hygiene The general goal is addressing FIXMEs from the previous PRs. Merging similar data structures (+ prerequisites for such merging), accounting for the fact that all `ExpnId`s have associated data in `HygieneData` now (less `Option`s). Also, some renaming. This should be the last renaming session in this area, I think. r? @matthewjasper

@matthewjasper

Continue refactoring resolve and hygiene The general goal is addressing FIXMEs from the previous PRs. Merging similar data structures (+ prerequisites for such merging), accounting for the fact that all `ExpnId`s have associated data in `HygieneData` now (less `Option`s). Also, some renaming. This should be the last renaming session in this area, I think. r? @matthewjasper

@matthewjasper

Continue refactoring resolve and hygiene The general goal is addressing FIXMEs from the previous PRs. Merging similar data structures (+ prerequisites for such merging), accounting for the fact that all `ExpnId`s have associated data in `HygieneData` now (less `Option`s). Also, some renaming. This should be the last renaming session in this area, I think. r? @matthewjasper

@ghost

Rollup of 7 pull requests Successful merges: - #62593 (Group all ABI tests.) - #63173 (Use libunwind from llvm-project submodule for musl targets) - #63535 (Continue refactoring resolve and hygiene) - #63539 (Suggest Rust 2018 on `<expr>.await` with no such field) - #63584 (libcore: more cleanups using `#![feature(associated_type_bounds)]`) - #63612 (Do not suggest `try_into` for base types inside of macro expansions) - #63615 (Fix typo in DoubleEndedIterator::nth_back doc) Failed merges: r? @ghost

bors · 2019-08-16T10:42:29Z

☔ The latest upstream changes (presumably #63627) made this pull request unmergeable. Please resolve the merge conflicts.

@matthewjasper

…ewjasper resolve: Properly integrate derives and `macro_rules` scopes So, ```rust #[derive(A, B)] struct S; m!(); ``` turns into something like ```rust struct S; A_placeholder!( struct S; ); B_placeholder!( struct S; ); m!(); ``` during expansion. And for `m!()` its "`macro_rules` scope" (aka "legacy scope") should point to the `B_placeholder` call rather than to the derive container `#[derive(A, B)]`. `fn build_reduced_graph` now makes sure the legacy scope points to the right thing. (It's still a mystery for me why this worked before rust-lang#63535.) Unfortunately, placeholders from derives are currently treated separately from placeholders from other macros and need to be passed as `extra_placeholders` rather than a part of the AST fragment. That's fixable, but I wanted to keep this PR more minimal to close the regression faster. Fixes rust-lang#63651 r? @matthewjasper

rust-highfive assigned matthewjasper Aug 13, 2019

petrochenkov mentioned this pull request Aug 13, 2019

Try to break resolve into more isolated parts #63400

Merged

matklad reviewed Aug 13, 2019

View reviewed changes

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2019

petrochenkov added 16 commits August 15, 2019 20:38

syntax_pos: Introduce a helper for checking whether a span comes from…

dfcbe75

… expansion

syntax_pos: NO_EXPANSION/SyntaxContext::empty() -> `SyntaxContext…

67d6ce4

…::root()` For consistency with `ExpnId::root`. Also introduce a helper `Span::with_root_ctxt` for creating spans with `SyntaxContext::root()` context

Ident::with_empty_ctxt -> Ident::with_dummy_span

6cb28b6

`Ident` has had a full span rather than just a `SyntaxContext` for a long time now.

hygiene: Remove Options from functions returning ExpnInfo

73dee25

The expansion info is not optional and should always exist

resolve: Do not "normalize away" trait/enum modules prematurely

cfbb60b

The previous approach was brittle - what would happen if `ParentScope` wasn't created by `invoc_parent_scope`? That's exactly the case for various uses of `ParentScope` in diagnostics and in built-in attribute validation.

resolve: Add ParentScope::default, eliminate dummy_parent_scope

1a1557c

Remove some unnecessary parameters from functions

resolve: Eliminate InvocationData

59dd07a

It was very similar to `ParentScope` and mostly could be replaced by it.

resolve: Make ParentScope Copy

ea68bc8

By allocating its derive paths on the resolver arena.

resolve: Privatize BuildReducedGraphVisitor

310ee4d

resolve: Add some comments to the main modules

aca1353

hygiene: Merge ExpnInfo and InternalExpnData

1a44773

syntax_pos: Remove the duplicate global edition

74190a5

It was introduced to avoid going through `hygiene_data`, but now it's read only once, when `ParseSess` is created, so going through a lock is ok.

hygiene: Merge a tiny bit of the "share expansion definition data" PR

650f19a

hygiene: ExpnInfo -> ExpnData

136db22

For naming consistency with everything else in this area

resolve: ParentScope::default -> ParentScope::module

c762773

petrochenkov force-pushed the expndata branch from f1ef7a9 to c762773 Compare August 15, 2019 18:04

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 Aug 15, 2019

Centril mentioned this pull request Aug 15, 2019

Rollup of 3 pull requests #63611

Closed

Centril mentioned this pull request Aug 15, 2019

Rollup of 7 pull requests #63617

Closed

Centril mentioned this pull request Aug 16, 2019

Rollup of 7 pull requests #63627

Merged

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 16, 2019

bors merged commit c762773 into rust-lang:master Aug 16, 2019

tesuji added a commit to tesuji/rust-clippy that referenced this pull request Aug 16, 2019

Rustup rust-lang/rust#63535

1bd8714

dtolnay mentioned this pull request Aug 17, 2019

Macro-generated macro can no longer be called #63651

Closed

petrochenkov mentioned this pull request Aug 17, 2019

resolve: Properly integrate derives and macro_rules scopes #63667

Merged

tesuji mentioned this pull request Aug 18, 2019

Recursive macro call inside concat macro fails to find macros referred to by $crate #63460

Closed

petrochenkov mentioned this pull request Aug 21, 2019

Fix nested eager expansions in arguments of format_args #63717

Merged

petrochenkov deleted the expndata branch February 22, 2025 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Continue refactoring resolve and hygiene#63535

Continue refactoring resolve and hygiene#63535
bors merged 16 commits intorust-lang:masterfrom
petrochenkov:expndata

petrochenkov commented Aug 13, 2019 •

edited

Loading

Uh oh!

matklad Aug 13, 2019

Uh oh!

matthewjasper Aug 15, 2019

Uh oh!

petrochenkov Aug 15, 2019

Uh oh!

matklad Aug 13, 2019

Uh oh!

matthewjasper commented Aug 15, 2019

Uh oh!

petrochenkov commented Aug 15, 2019

Uh oh!

bors commented Aug 15, 2019

Uh oh!

bors commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

petrochenkov commented Aug 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matklad Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

matthewjasper Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkov Aug 15, 2019

Choose a reason for hiding this comment

Uh oh!

matklad Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

matthewjasper commented Aug 15, 2019

Uh oh!

petrochenkov commented Aug 15, 2019

Uh oh!

bors commented Aug 15, 2019

Uh oh!

bors commented Aug 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

petrochenkov commented Aug 13, 2019 •

edited

Loading