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.

Sign up for GitHub

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

rustc_parse: Remove optimization for 0-length streams in collect_tokens #78736

Merged
merged 1 commit into from
Nov 14, 2020
+5 −9
Merged

rustc_parse: Remove optimization for 0-length streams in collect_tokens #78736

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions compiler/rustc_parse/src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,8 +1180,7 @@ impl<'a> Parser<'a> {
/// Records all tokens consumed by the provided callback,
/// including the current token. These tokens are collected
/// into a `LazyTokenStream`, and returned along with the result
/// of the callback. The returned `LazyTokenStream` will be `None`
/// if not tokens were captured.
/// of the callback.
///
/// Note: If your callback consumes an opening delimiter
/// (including the case where you call `collect_tokens`
Expand All @@ -1203,17 +1202,14 @@ impl<'a> Parser<'a> {

let ret = f(self)?;

// We didn't capture any tokens
let num_calls = self.token_cursor.num_next_calls - cursor_snapshot.num_next_calls;
if num_calls == 0 {
return Ok((ret, None));
}

// Produces a `TokenStream` on-demand. Using `cursor_snapshot`
// and `num_calls`, we can reconstruct the `TokenStream` seen
// by the callback. This allows us to avoid producing a `TokenStream`
// if it is never needed - for example, a captured `macro_rules!`
// argument that is never passed to a proc macro.
// In practice token stream creation happens rarely compared to
// calls to `collect_tokens` (see some statistics in #78736),
// so we are doing as little up-front work as possible.
//
// This also makes `Parser` very cheap to clone, since
// there is no intermediate collection buffer to clone.
Expand Down Expand Up @@ -1247,8 +1243,8 @@ impl<'a> Parser<'a> {

let lazy_impl = LazyTokenStreamImpl {
start_token,
num_calls: self.token_cursor.num_next_calls - cursor_snapshot.num_next_calls,
cursor_snapshot,
num_calls,
desugar_doc_comments: self.desugar_doc_comments,
};
Ok((ret, Some(LazyTokenStream::new(lazy_impl))))
Copy link
Member

Choose a reason for hiding this comment

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

Do you still want collect_tokens to return an Option<LazyTokenStream>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to include the

let (thing, tokens) = self.collect_tokens(...);
if thing.tokens.is_none() {
    thing.tokens = tokens;
}

logic into collect_tokens itself instead, but that's probably a task for another PR.

Expand Down