-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Some minor cleanup and improvements to unicode.py #30695
Conversation
The methods related to char width are dead code since 464cdff; remove them.
Make unicode.py generate a tables.rs which is more conformant to usual Rust formatting (as per `rustfmt`).
Do not hand-code `Result::ok` or `cmp` in tables.rs.
As mentioned in rust-lang#29734, the range comparison closure can be improved. The LLVM IR and the assembly from the new version are much simpler and unfortunately we cannot rely on the compiler to optimise this much, as it would need to know that `lo <= hi`. Besides from simpler code, there might also be a performance advantage, although it is unlikely to appear on benchmarks, as we are doing a binary search, which should always involve few comparisons. The code is available on the playpen for ease of comparison: http://is.gd/4raMmH
with a new version generated by src/etc/unicode.py.
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
and the associated update of tables.rs The last commit is related to my comment to #29734.
💔 Test failed - auto-mac-64-opt |
@bors retry |
⚡ Previous build results for auto-linux-cross-opt, auto-linux-musl-64-opt, auto-win-msvc-32-opt are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-64-opt... |
and the associated update of tables.rs
The last commit is related to my comment to #29734.