Skip to content
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

Add some utilities to libsyntax #50192

Merged
merged 4 commits into from
Apr 27, 2018

Conversation

sapphire-arches
Copy link
Contributor

Adds a few functions to Mark and Span that I found useful in an upcoming refactor of NLL region error reporting. Also includes some new documentation based on my discussion with @jseyfried on IRC.

r? @jseyfried

... and reimplement proc_macro::Span::parent using it. This function turns out
to be useful in the compiler as well
This is useful when trying to compute when something is lexically before
something else, but they aren't necessarily in the same SyntaxContext
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2018
Copy link
Contributor

@jseyfried jseyfried left a comment

Choose a reason for hiding this comment

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

Nice! r=me modulo comments (nits optional)

@@ -270,7 +270,7 @@ impl Span {
/// `self` was generated from, if any.
#[unstable(feature = "proc_macro", issue = "38356")]
pub fn parent(&self) -> Option<Span> {
self.0.ctxt().outer().expn_info().map(|i| Span(i.call_site))
self.0.parent().map(|x| { Span(x) })
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: .map(Span)

/// assert!(a.is_descendant_of(lub))
/// assert!(b.is_descendant_of(lub))
/// ```
pub fn lub(mut a: Mark, mut b: Mark) -> Mark {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe least_ancestor for clarity and symmetry with the is_descendant_of? (I'd probably call this join if it were math)

pub fn lub(mut a: Mark, mut b: Mark) -> Mark {
HygieneData::with(|data| {
// Compute the path from a to the root
let mut a_path = FxHashSet::<Mark>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, O(n) :)


// While the path from b to the root hasn't intersected, move up the tree
while !a_path.contains(&b) {
b = data.marks[b.0 as usize].parent;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: two spaces after =

/// and we have a SyntaxContext that is referring to something declared by an invocation
/// of g (call it g1), calling remove_mark will result in the SyntaxContext for the
/// invocation of f that created g1.
/// Returns the mark that was removed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

@@ -291,6 +291,12 @@ impl Span {
self.ctxt().outer().expn_info().map(|info| info.call_site.source_callsite()).unwrap_or(self)
}

/// The `Span for the tokens in the previous macro expansion from which `self` was generated,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: `Span

@sapphire-arches
Copy link
Contributor Author

@jseyfried I've pushed fixes for the nits, and changed the name of lub to least_ancestor. Definitely a better name =)

@jseyfried
Copy link
Contributor

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Apr 27, 2018

📌 Commit 73e0c1e has been approved by jseyfried

@bors 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 Apr 27, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Apr 27, 2018
…=jseyfried

Add some utilities to `libsyntax`

Adds a few functions to `Mark` and `Span` that I found useful in an upcoming refactor of NLL region error reporting. Also includes some new documentation based on my discussion with @jseyfried on IRC.

r? @jseyfried
bors added a commit that referenced this pull request Apr 27, 2018
Rollup of 9 pull requests

Successful merges:

 - #49858 (std: Mark `ptr::Unique` with `#[doc(hidden)]`)
 - #49968 (Stabilize dyn trait)
 - #50192 (Add some utilities to `libsyntax`)
 - #50251 (rustc: Disable threads in LLD for wasm)
 - #50263 (rustc: Emit `uwtable` for allocator shims)
 - #50269 (Update `parking_lot` dependencies)
 - #50273 (Allow #[inline] on closures)
 - #50284 (fix search load page failure)
 - #50257 (Don't ICE on tuple struct ctor with incorrect arg count)

Failed merges:
@bors bors merged commit 73e0c1e into rust-lang:master Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants