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::Group::span_open and span_close #53902

Merged
merged 3 commits into from Sep 9, 2018

Conversation

Projects
None yet
6 participants
@dtolnay
Member

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 group.span_open() and group.span_close() which access the Span associated with just the opening delimiter and just the closing delimiter of the group. Relevant to Syn as we implement real error messages for when parsing fails in a procedural macro: dtolnay/syn#476.

  impl Group {
      fn span(&self) -> Span;
+     fn span_open(&self) -> Span;
+     fn span_close(&self) -> Span;
  }

Fixes #48187
r? @alexcrichton

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Sep 2, 2018

Member

I'd be fine adding these, but I'd prefer to add them as unstable instead of initially insta-stable.

Member

alexcrichton commented Sep 2, 2018

I'd be fine adding these, but I'd prefer to add them as unstable instead of initially insta-stable.

proc_macro::Group::span_open and span_close
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 group.span_open() and group.span_close() which access
the Span associated with just the opening delimiter and just the closing
delimiter of the group. Relevant to Syn as we implement real error
messages for when parsing fails in a procedural macro.
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Sep 3, 2018

Member

r? @petrochenkov

I'm personally comfortable with this as-is, but I think you've got further comments!

Member

alexcrichton commented Sep 3, 2018

r? @petrochenkov

I'm personally comfortable with this as-is, but I think you've got further comments!

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay Sep 3, 2018

Member

Closing in favor of #53930, span.len() and span.subspan(lo..hi) as discussed in #53904 (comment).

Member

dtolnay commented Sep 3, 2018

Closing in favor of #53930, span.len() and span.subspan(lo..hi) as discussed in #53904 (comment).

@dtolnay dtolnay closed this Sep 3, 2018

@dtolnay dtolnay deleted the dtolnay:group branch Sep 3, 2018

@dtolnay dtolnay restored the dtolnay:group branch Sep 9, 2018

@dtolnay dtolnay reopened this Sep 9, 2018

@dtolnay

This comment has been minimized.

Show comment
Hide comment
@dtolnay

dtolnay Sep 9, 2018

Member

Updated to include #53930 (comment).

Member

dtolnay commented Sep 9, 2018

Updated to include #53930 (comment).

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov Sep 9, 2018

Contributor

@bors r+
This is a small API addition and can be deprecated if necessary.
I still think there's nothing wrong in guaranteeing that a Span is an arbitrary sliceable (possibly UTF-8 encoded) continuous byte range and providing the API from #53930.

Contributor

petrochenkov commented Sep 9, 2018

@bors r+
This is a small API addition and can be deprecated if necessary.
I still think there's nothing wrong in guaranteeing that a Span is an arbitrary sliceable (possibly UTF-8 encoded) continuous byte range and providing the API from #53930.

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 9, 2018

Contributor

📌 Commit 57d6ada has been approved by petrochenkov

Contributor

bors commented Sep 9, 2018

📌 Commit 57d6ada has been approved by petrochenkov

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 9, 2018

Contributor

⌛️ Testing commit 57d6ada with merge 40fc8ba...

Contributor

bors commented Sep 9, 2018

⌛️ Testing commit 57d6ada with merge 40fc8ba...

bors added a commit that referenced this pull request Sep 9, 2018

Auto merge of #53902 - dtolnay:group, r=petrochenkov
proc_macro::Group::span_open and span_close

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:

```rust
mod m {
    type T =
}
```

```console
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 `group.span_open()` and `group.span_close()` which access the Span associated with just the opening delimiter and just the closing delimiter of the group. Relevant to Syn as we implement real error messages for when parsing fails in a procedural macro: dtolnay/syn#476.

```diff
  impl Group {
      fn span(&self) -> Span;
+     fn span_open(&self) -> Span;
+     fn span_close(&self) -> Span;
  }
```

Fixes #48187
r? @alexcrichton
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Sep 9, 2018

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing 40fc8ba to master...

Contributor

bors commented Sep 9, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing 40fc8ba to master...

@bors bors merged commit 57d6ada into rust-lang:master Sep 9, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@dtolnay dtolnay deleted the dtolnay:group branch Sep 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment