Skip to content

Refactored new ScopeData for improved abstraction. #19033

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

Merged

Conversation

pnkfelix
Copy link
Member

(Previously, scopes were solely identified with NodeId's; this
refactoring prepares for a future where that does not hold.)

Ground work for a proper fix to #8861.

@pnkfelix
Copy link
Member Author

r? @nikomatsakis (but please feel free to reassign; so much of this is mechanical that it probably could be "r? anyone")

@nikomatsakis
Copy link
Contributor

r+ (deliberately did not put on the commit)

Only thing is that I think we should rename ScopeData, which to me suggests "data about scopes" and not "a single scope". On IRC @pnkfelix and I settled on CodeRegion; I'm happy with that or RegionScope or even Extent.

I also think we should probably find a shorter way to go from a node-id to a CodeRegion/RegionScope/Extent, because region::Foo::from_node_id(...) is quite long. Directly importing the name would help, so at least one could write Extent::from_node_id(...), or perhaps defining a free fn in region, so one could write region::extent(node-id). I suspect many of these lines are > 100 characters (I see that travis is failing, haven't investigated, but I bet that is why).

@@ -76,11 +117,11 @@ The region maps encode information about region relationships.
for dynamic checks and/or arbitrary amounts of stack space.
*/
pub struct RegionMaps {
scope_map: RefCell<NodeMap<ast::NodeId>>,
var_map: RefCell<NodeMap<ast::NodeId>>,
scope_map: RefCell<HashMap<ScopeData, ScopeData>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should be a FnvHashMap.

@pnkfelix pnkfelix force-pushed the fsk-introduce-scopedata-via-refactor branch 3 times, most recently from 48f24d5 to dd95fa9 Compare November 19, 2014 16:02
(Previously, statically identifiable scopes/regions were solely
identified with NodeId's; this refactoring prepares for a future
where that 1:1 correspondence does not hold.)
@pnkfelix pnkfelix force-pushed the fsk-introduce-scopedata-via-refactor branch from dd95fa9 to 5ff9087 Compare November 20, 2014 12:10
bors added a commit that referenced this pull request Nov 20, 2014
…ctor, r=nikomatsakis

(Previously, scopes were solely identified with NodeId's; this
refactoring prepares for a future where that does not hold.)

Ground work for a proper fix to #8861.
@bors bors closed this Nov 20, 2014
@bors bors merged commit 5ff9087 into rust-lang:master Nov 20, 2014
lnicola pushed a commit to lnicola/rust that referenced this pull request Jan 27, 2025
fix: Make proc_macro span's line & column 1-indexed, as documented
# 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