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

proc_macro::Span::at_start and at_end #53904

Closed
wants to merge 1 commit into from
Closed

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Sep 2, 2018

Before this addition, every delimited group like (...) [...] {...} has only a single Span that covers the full source location from opening delimiter to closing delimiter. This makes it impossible for a procedural macro to trigger an error pointing to just the opening or closing delimiter. The Rust compiler does not seem to have the same limitation:

mod m {
    type T =
}
error: expected type, found `}`
 --> src/main.rs:3:1
  |
3 | }
  | ^

On that same input, a procedural macro would be forced to trigger the error on the last token inside the block, on the entire block, or on the next token after the block, none of which is really what you want for an error like above.

This commit adds span.at_start() and span.at_end() which access the Span associated with just the first byte (for groups, this is the opening delimiter) and just the last byte (for groups, the closing delimiter). Relevant to Syn as we implement real error messages for when parsing fails in a procedural macro: dtolnay/syn#476.

  impl Span {
+     fn at_start(self) -> Span;
+     fn at_end(self) -> Span;
  }

Fixes #48187
r? @petrochenkov

Before this addition, every delimited group like (...) [...] {...} has
only a single Span that covers the full source location from opening
delimiter to closing delimiter. This makes it impossible for a
procedural macro to trigger an error pointing to just the opening or
closing delimiter. The Rust compiler does not seem to have the same
limitation:

    mod m {
        type T =
    }

    error: expected type, found `}`
     --> src/main.rs:3:1
      |
    3 | }
      | ^

On that same input, a procedural macro would be forced to trigger the
error on the last token inside the block, on the entire block, or on the
next token after the block, none of which is really what you want for an
error like above.

This commit adds span.at_start() and span.at_end() which access the Span
associated with just the first byte (opening delimiter) and just the
last byte (closing delimiter) of the group. Relevant to Syn as we
implement real error messages for when parsing fails in a procedural
macro.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 2, 2018
/// pub fn at_start(self) -> Span {
/// ^
/// ```
#[stable(feature = "proc_macro_span_start_end", since = "1.30.0")]
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something more generic like

pub fn sub_span(self, begin: usize, end: usize) -> Span

and, optionally,

pub fn length(&self) -> usize

This would then also solve my use case of pointing an error to part of a template string like the compiler can do

  |
6 |     format!("{.}", 1);
  |               ^ expected `}` in format string

Copy link
Contributor

Choose a reason for hiding this comment

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

@parched
Yeah, len() + Index<RangeBounds<usize>> for spans, basically - span[a .. b], span[a..].
Except not Index exactly, because it must return a reference.

That would be a small, self-contained, but powerful API.
I like it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a safer addition than combining >1 spans or arbitrary arithmetic with lo/hi for spans, because we have only one hygienic context that's inherited, and the input span is valid and continuous, so the resulting span will be valid as well.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 3, 2018

Closing in favor of span.len() and span.subspan(lo..hi) in #53930.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants