-
Notifications
You must be signed in to change notification settings - Fork 13.3k
resolve: cleanup and groundwork for resolving the AST #33232
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
Conversation
…ath` to return a `NameBinding` instead of a `Def`
Refactor `ty::Visibility` methods to use a new trait `NodeIdTree` instead of the ast map.
loop { | ||
if node_ancestor == ancestor { return true } | ||
let node_ancestor_parent = self.get_module_parent(node_ancestor); | ||
if node_ancestor_parent == node_ancestor { return false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style nit: spread these if
s over three lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do
Nice! r+ with the style nits addressed. |
@nrc thanks for the quick review! I addressed the style nits in the above commit. |
let module_parent = match self.get_nearest_normal_module_parent(module) { | ||
Some(parent) => parent, | ||
None => return false, | ||
}; | ||
module = module_parent; | ||
} | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer true
rather than return true;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and in the other function too
28a2f7e
to
ac26419
Compare
Whoops -- didn't notice that when mechanically refactoring, fixed. |
@bors: r+ |
📌 Commit ac26419 has been approved by |
⌛ Testing commit ac26419 with merge cc1b031... |
💔 Test failed - auto-win-msvc-64-opt |
@bors: retry On Thu, Apr 28, 2016 at 11:34 PM, bors [email protected] wrote:
|
⌛ Testing commit ac26419 with merge b2bc29d... |
💔 Test failed - auto-mac-32-opt |
@bors: retry On Friday, April 29, 2016, bors [email protected] wrote:
|
resolve: cleanup and groundwork for resolving the AST Cleanup `resolve` and refactor away uses of the hir map (incorrectly named `ast_map`). r? @nrc
Cleanup
resolve
and refactor away uses of the hir map (incorrectly namedast_map
).r? @nrc