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

Do not merge: Convert StringReader to an iterator #63321

Closed
wants to merge 4 commits into from

Conversation

matklad
Copy link
Member

@matklad matklad commented Aug 6, 2019

As a follow-up to #63017 (so, take a look at the last commit only), I've tried to convert the StringReader to an Iterator. It is certainly possible, but I think we shouldn't do this, and hence this PR is a "negative result", for posterity.

The reasons are:

  • It makes little sense without removal of explicit Eof token, and Eof is used by other code as well (notably, the tt cursor)
  • eof token nicely carries a span with it
  • (most important, imo): code that consumes the lexer looks better with Eof token, as you can write shorter matches and call methods without .as_ref().map dance. (Note similarities with Eof char in rustc_lexer)

cc @petrochenkov, @eddyb

@matklad
Copy link
Member Author

matklad commented Aug 6, 2019

So, closing as a bad idea :)

@matklad matklad closed this Aug 6, 2019
@rust-highfive
Copy link
Collaborator

Your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-08-06T08:28:04.8942950Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-06T08:28:04.9158898Z ##[command]git config gc.auto 0
2019-08-06T08:28:04.9213198Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-06T08:28:04.9271229Z ##[command]git config --get-all http.proxy
2019-08-06T08:28:04.9426255Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63321/merge:refs/remotes/pull/63321/merge
2019-08-06T08:28:06.1158120Z fatal: couldn't find remote ref refs/pull/63321/merge
2019-08-06T08:28:06.2161685Z ##[warning]Git fetch failed with exit code 128, back off 1.005 seconds before retry.
2019-08-06T08:28:07.1540254Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63321/merge:refs/remotes/pull/63321/merge
2019-08-06T08:28:07.7379889Z fatal: couldn't find remote ref refs/pull/63321/merge
2019-08-06T08:28:07.8001641Z ##[warning]Git fetch failed with exit code 128, back off 3.718 seconds before retry.
2019-08-06T08:28:11.4605318Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/63321/merge:refs/remotes/pull/63321/merge
2019-08-06T08:28:12.1251925Z fatal: couldn't find remote ref refs/pull/63321/merge
2019-08-06T08:28:12.1780469Z ##[error]Git fetch failed with exit code: 128
2019-08-06T08:28:12.2064301Z ##[section]Starting: Checkout
2019-08-06T08:28:12.2065850Z ==============================================================================
2019-08-06T08:28:12.2065903Z Task         : Get sources
2019-08-06T08:28:12.2065947Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants