-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Streamline StringReader::bump
#50566
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Thanks! I hope we didn't miss any off-by-one errors here. |
📌 Commit f04510d has been approved by |
⌛ Testing commit f04510de8ed288dd1af513b6f2ededa86c83c843 with merge 7f312754402735e825842b33d9222d8eb6abf4f9... |
💔 Test failed - status-travis |
The job Click to expand the log.
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 |
1 similar comment
The job Click to expand the log.
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 |
@bors retry |
⌛ Testing commit f04510de8ed288dd1af513b6f2ededa86c83c843 with merge 63dbff88a50864891f68f27e1d960deedaca87f7... |
💔 Test failed - status-appveyor |
col -= 1; | ||
let len = ch.len_utf8(); | ||
cursor += len; | ||
col_pos -= len; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
---- [pretty] pretty\block-comment-wchar.rs stdout ----
error: pretty-printing failed in round 0 revision None
status: exit code: 101
command: PATH="C:\projects\rust\build\x86_64-pc-windows-msvc\stage2\bin;C:\projects\rust\build\x86_64-pc-windows-msvc\llvm\build\bin;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.14393.0\x64;C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\amd64;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-tools\x86_64-pc-windows-msvc\release\deps;C:\projects\rust\build\x86_64-pc-windows-msvc\stage0-sysroot\lib\rustlib\x86_64-pc-windows-msvc\lib;C:\Program Files (x86)\Inno Setup 5;C:\Python27;C:\msys64\mingw64\bin;C:\msys64\usr\bin;C:\Perl\site\bin;C:\Perl\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Program Files\7-Zip;C:\Program Files\Microsoft\Web Platform Installer;C:\Tools\GitVersion;C:\Tools\PsTools;C:\Program Files\Git LFS;C:\Program Files (x86)\Subversion\bin;C:\Program Files\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\110\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn;C:\Program Files\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\Tools\Binn\ManagementStudio;C:\Tools\WebDriver;C:\Program Files (x86)\Microsoft SDKs\TypeScript\1.4;C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\IDE\PrivateAssemblies;C:\Program Files (x86)\Microsoft SDKs\Azure\CLI\wbin;C:\Ruby193\bin;C:\Tools\NUnit\bin;C:\Tools\xUnit;C:\Tools\MSpec;C:\Tools\Coverity\bin;C:\Program Files (x86)\CMake\bin;C:\go\bin;C:\Program Files\Java\jdk1.8.0\bin;C:\Python27;C:\Program Files\nodejs;C:\Program Files (x86)\iojs;C:\Program Files\iojs;C:\Users\appveyor\AppData\Roaming\npm;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\MSBuild\14.0\Bin;C:\Tools\NuGet;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\CommonExtensions\Microsoft\TestWindow;C:\Program Files\Microsoft DNX\Dnvm;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\Tools\Binn;C:\Program Files (x86)\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\110\DTS\Binn;C:\Program Files (x86)\Microsoft SQL Server\120\DTS\Binn;C:\Program Files (x86)\Apache\Maven\bin;C:\Python27\Scripts;C:\Tools\NUnit3;C:\Program Files\Mercurial;C:\Program Files\LLVM\bin;C:\Program Files\dotnet;C:\Tools\curl\bin;C:\Program Files\Amazon\AWSCLI;C:\Program Files (x86)\Microsoft SQL Server\140\DTS\Binn;C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\Extensions\Microsoft\SQLDB\DAC\140;C:\Tools\vcpkg;C:\Program Files (x86)\Microsoft SQL Server\140\Tools\Binn;C:\Program Files\Microsoft SQL Server\140\Tools\Binn;C:\Program Files\Microsoft SQL Server\140\DTS\Binn;C:\Program Files\PowerShell\6.0.0;C:\Program Files\erl9.2\bin;C:\Program Files (x86)\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\usr\bin;C:\Program Files (x86)\Yarn\bin;C:\Program Files (x86)\NSIS;C:\Tools\Octopus;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\ProgramData\chocolatey\bin;C:\Users\appveyor\AppData\Roaming\npm;C:\Users\appveyor\AppData\Local\Yarn\bin;C:\Program Files\AppVeyor\BuildAgent;C:\projects\rust;C:\projects\rust\handle" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\bin\\rustc.exe" "-" "-Z" "unpretty=normal" "--target" "x86_64-pc-windows-msvc" "-L" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\test\\pretty\\block-comment-wchar.stage2-x86_64-pc-windows-msvc.pretty.aux" "-Crpath" "-O" "-Zunstable-options" "-Lnative=C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\native\\rust-test-helpers"
stdout:
------------------------------------------
------------------------------------------
stderr:
------------------------------------------
thread 'main' panicked at 'attempt to subtract with overflow', libsyntax\parse\lexer\comments.rs:215:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.27.0-dev running on x86_64-pc-windows-msvc
note: compiler flags: -Z unpretty=normal -Z unstable-options -C rpath
------------------------------------------
thread '[pretty] pretty\block-comment-wchar.rs' panicked at 'explicit panic', tools\compiletest\src\runtest.rs:3033:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
[pretty] pretty\block-comment-wchar.rs
test result: FAILED. 46 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
Because `bump()` is hot.
This patch removes the "old"/"new" names in favour of "foo"/"next_foo", which matches the field names. It also moves the setting of `self.{ch,pos,next_pos}` in the common case to the end, so that the meaning of "foo"/"next_foo" is consistent until the end.
- `source_text` becomes `src`, matching `FileMap::src`. - `byte_offset()` becomes `src_index()`, which makes it clearer that it's an index into `src`. (Likewise for variables containing `byte_offset` in their name.) This function also now returns a `usize` instead of a `BytePos`, because every callsite immediately converted the `BytePos` to a `usize`.
It's silly for a hot function like `bump()` to have such an expensive bounds check. This patch replaces terminator with `end_src_index`. Note that the `self.terminator` check in `is_eof()` wasn't necessary because of the way `StringReader` is initialized.
@bors r+ |
📌 Commit e913d69 has been approved by |
⌛ Testing commit e913d69 with merge a3412980525c11719c991b27b003e0d2c4a44e0f... |
💔 Test failed - status-travis |
The job Click to expand the log.
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 |
1 similar comment
The job Click to expand the log.
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 |
⌛ Testing commit e913d69 with merge 9a095e285785af647046c46a5e55edb12ca1bbb0... |
@bors p=15 |
💔 Test failed - status-appveyor |
@bors retry |
Streamline `StringReader::bump` These patches make `bump` smaller and nicer. They speed up most runs for coercions and tuple-stress by 1--3%.
☀️ Test successful - status-appveyor, status-travis |
These patches make
bump
smaller and nicer. They speed up most runs for coercions and tuple-stress by 1--3%.