-
Notifications
You must be signed in to change notification settings - Fork 13.4k
syntax: use an index in CodeMap instead of Gc for ExpnInfo. #17314
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
ad7ec1e
to
8b7847c
Compare
pub fn record_expansion(&self, expn_info: ExpnInfo) -> ExpnId { | ||
let mut expansions = self.expansions.borrow_mut(); | ||
expansions.push(expn_info); | ||
ExpnId(expansions.len().to_u32().unwrap() - 1) |
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.
as u32
? (If you're doing this for verification/assertion purposes, .expect("too many ExpnInfo's")
would be better.)
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.
Also, minor point: this will break all the NO_EXPANSION
s if 232 - 1 elements are inserted.
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.
No, it would break if 232 elements are inserted, which is what I'm checking for (I will switch to expect
).
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.
Oh, of course.
4f26f69
to
0db5292
Compare
loop { | ||
let expn_info = self.codemap().with_expn_info(expn_id, |ei| { | ||
ei.map(|ei| (ei.call_site, ei.callee.name.as_slice() == "include")) | ||
}) |
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.
plop a semicolon here.
Also:
|
} | ||
Ok(()) | ||
let cs = try!(cm.with_expn_info(sp.expn_id, |expn_info| match expn_info { | ||
Some(ei) => { |
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.
Could be Ok(expn_info.map(|ei| { ... }))
rather than a match
.
No description provided.