Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Perform name resolution before and during ast->hir lowering #33443

Merged
merged 10 commits into from
May 10, 2016

Conversation

jseyfried
Copy link
Contributor

This PR performs name resolution before and during ast->hir lowering instead of in phase 3.
r? @nrc

} else {
self.visit_block(block);
}

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 factor out this code dup?

Copy link
Contributor Author

@jseyfried jseyfried May 6, 2016

Choose a reason for hiding this comment

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

Done

@nrc
Copy link
Member

nrc commented May 6, 2016

A few comments inline (some of which GitHub seems to be hiding due to already being out of date some how). r=me with those addressed.

@KalitaAlexey
Copy link
Contributor

What purpose of it?

@jseyfried
Copy link
Contributor Author

jseyfried commented May 6, 2016

@KalitaAlexey
Some improvements and a feature require it. Specifically, we want to

  • Improve the hir for patterns (hir::PatKind). In particular,
    • Only use the Ident variant for bindings. Currently, it is also used for single identifier unit struct and const patterns, which cannot be distinguished from bindings before resolution.
    • Replace the variant Path with a variant for unit structs/variants a variant for constants.
    • Represent all associated constants with the QPath variant.
  • Store PathResolutions (basically, Defs except for associated constants/types) in the hir itself instead of in def_map.
  • Implement RFC 1560. We want to be able to use qualified paths in macro invocations and to import macros like items (i.e. with use).

@KalitaAlexey
Copy link
Contributor

@jseyfried Thanks.

// We must keep the set of definitions up to date as we add nodes that weren't in the AST.
fn definitions(&mut self) -> &mut Definitions;
// This should only return `None` during testing.
fn definitions(&mut self) -> Option<&mut Definitions>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could further refactor this into local_def_id (which would return a dummy DefId during testing) and crate_def_with_parent (which would do nothing during testing).

@jseyfried jseyfried force-pushed the resolve_ast branch 2 times, most recently from 46d0fb8 to 6ebe69c Compare May 8, 2016 05:34
@bors
Copy link
Collaborator

bors commented May 8, 2016

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

@jseyfried jseyfried force-pushed the resolve_ast branch 3 times, most recently from 8b5bd6c to e4f56e5 Compare May 9, 2016 09:03
@jseyfried
Copy link
Contributor Author

@nrc I finally finished rebasing and fixing fallout in tests and rustdoc. Could you review the last three commits?

@nrc
Copy link
Member

nrc commented May 9, 2016

@bors: r+

@bors
Copy link
Collaborator

bors commented May 9, 2016

📌 Commit 805666a has been approved by nrc

@bors
Copy link
Collaborator

bors commented May 10, 2016

⌛ Testing commit 805666a with merge a4d2424...

bors added a commit that referenced this pull request May 10, 2016
Perform name resolution before and during ast->hir lowering

This PR performs name resolution before and during ast->hir lowering instead of in phase 3.
r? @nrc
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants