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

Add more logarithm constants #50539

Merged
merged 1 commit into from
May 9, 2018
Merged

Add more logarithm constants #50539

merged 1 commit into from
May 9, 2018

Conversation

clarfonthey
Copy link
Contributor

Right now, we have ln(2) and ln(10), but only log2(e) and log10(e). This also adds log2(10) and log10(2) for consistency.

@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2018
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Could you share some example use cases that require these constants? I think I've never seen log10(2) before.

@clarfonthey
Copy link
Contributor Author

@dtolnay log2(e) and log10(e) are used to convert from binary/decimal logs to natural logs, and log2(10) and log10(2) are used to convert between binary/decimal logs. These are actually useful in string conversion, as you can use log2(10) to get an upper bound on the length of an integer string and log10(2) to get an upper bound on the number of bits required, given the length of a string.

Obviously these aren't super useful in every context, but I'd argue that log2(10) and log10(2) are more useful than log2(e) and log10(e), which are currently included.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (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.
[00:03:22]    Compiling std v0.0.0 (file:///checkout/src/libstd)
[00:03:23] error[E0547]: missing 'issue'
[00:03:23]    --> libcore/num/f32.rs:132:5
[00:03:23]     |
[00:03:23] 132 |     #[unstable(feature = "extra_log_consts")]
[00:03:23] 
[00:03:23] error[E0547]: missing 'issue'
[00:03:23]    --> libcore/num/f32.rs:140:5
[00:03:23]     |
[00:03:23]     |
[00:03:23] 140 |     #[unstable(feature = "extra_log_consts")]
[00:03:23] 
[00:03:23] error[E0547]: missing 'issue'
[00:03:23]    --> libcore/num/f64.rs:128:5
[00:03:23]     |
[00:03:23]     |
[00:03:23] 128 |     #[unstable(feature = "extra_log_consts")]
[00:03:23] 
[00:03:23] error[E0547]: missing 'issue'
[00:03:23]    --> libcore/num/f64.rs:136:5
[00:03:23]     |
[00:03:23]     |
[00:03:23] 136 |     #[unstable(feature = "extra_log_consts")]
[00:03:23] 
[00:03:26]    Compiling compiler_builtins v0.0.0 (file:///checkout/src/rustc/compiler_builtins_shim)
[00:03:26]    Compiling cmake v0.1.30
[00:03:26]    Compiling alloc_jemalloc v0.0.0 (file:///checkout/src/liballoc_jemalloc)
---
[00:03:36] Caused by:
[00:03:36]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name core libcore/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,link -C opt-level=3 -C metadata=fb1e36473ec4786e -C extra-filename=-fb1e36473ec4786e --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-std/release/deps` (exit code: 101)
[00:03:36] warning: build failed, waiting for other jobs to finish...
[00:03:45] error: build failed
[00:03:45] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:03:45] expected success, got: exit code: 101
[00:03:45] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1091:9
[00:03:45] travis_fold:end:stage0-std

[00:03:45] travis_time:end:stage0-std:start=1525797730516430972,finish=1525797757743978115,duration=27227547143


[00:03:45] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:45] Build completed unsuccessfully in 0:00:29
[00:03:45] Makefile:79: recipe for target 'tidy' failed
[00:03:45] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:34322484
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

@dtolnay
Copy link
Member

dtolnay commented May 8, 2018

Looking for some precedent I found that Go provides only the four we already have while D provides all six.

I would be on board with landing these unstable. Please file a tracking issue and include the issue number in the PR. Thanks!

@clarfonthey
Copy link
Contributor Author

Done!

@@ -128,10 +128,18 @@ pub mod consts {
#[stable(feature = "rust1", since = "1.0.0")]
pub const LOG2_E: f32 = 1.44269504088896340735992468100189214_f32;

/// log<sub>2</sub>(10)
#[unstable(feature = "extra_log_consts", issue = "50539")]
Copy link
Member

Choose a reason for hiding this comment

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

Please use the tracking issue number and not the PR number.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake; that's fixed now.

@dtolnay
Copy link
Member

dtolnay commented May 8, 2018

@bors r+

@bors
Copy link
Contributor

bors commented May 8, 2018

📌 Commit 23b6e46 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 8, 2018
kennytm added a commit to kennytm/rust that referenced this pull request May 9, 2018
Add more logarithm constants

Right now, we have `ln(2)` and `ln(10)`, but only `log2(e)` and `log10(e)`. This also adds `log2(10)` and `log10(2)` for consistency.
bors added a commit that referenced this pull request May 9, 2018
Rollup of 11 pull requests

Successful merges:

 - #49988 (Mention Result<!, E> in never docs.)
 - #50148 (turn `ManuallyDrop::new` into a constant function)
 - #50456 (Update the Cargo submodule)
 - #50460 (Make `String::new()` const)
 - #50464 (Remove some transmutes)
 - #50505 (Added regression function match value test)
 - #50511 (Add some explanations for #[must_use])
 - #50525 (Optimize string handling in lit_token().)
 - #50527 (Cleanup a `use` in a raw_vec test)
 - #50539 (Add more logarithm constants)
 - #49523 (Update RELEASES.md for 1.26.0)

Failed merges:
@bors bors merged commit 23b6e46 into rust-lang:master May 9, 2018
@clarfonthey clarfonthey deleted the log_const branch June 22, 2018 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants