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

Jump to bottom

Document results of non-positive logarithms #137357

Merged
merged 1 commit into from
Mar 21, 2025
Merged

Document results of non-positive logarithms #137357

merged 1 commit into from
Mar 21, 2025
+200 −0

Conversation

syvb
Copy link
Contributor

@syvb syvb commented Feb 21, 2025 •
edited by tgross35
Loading

The integer versions of logarithm functions panic on non-positive numbers. The floating point versions have different, undocumented behaviour (-inf on 0, NaN on <0). This PR documents that.

try-job: aarch64-gnu

@rustbot
Copy link
Collaborator

rustbot commented Feb 21, 2025

r? @Noratrieb

rustbot has assigned @Noratrieb.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 21, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@tgross35
Copy link
Contributor

This seems reasonable enough. Snips from 754 to back up the behavior, from section 9's exception table:

log log2 log10: x = 0: divideByZero; x < 0: invalid operation

logp1 log2p1 log10p1: x = −1: divideByZero; x < −1: invalid operation; underflow

9.2.1 special values:

For the operations log, log2, and log10, f (±0) is −∞ and signals the divideByZero exception, and f (1) is +0. For the operations logp1, log2p1, and log10p1, f (−1) is −∞ and signals the divideByZero exception.

And then just confirming that FE_INVALID returns NaN:

invalid operation: For all operations, signaling NaN operands shall signal the invalid operation exception.

Would you mind adding a note to ln_1p since it has similar domain conditions?

@juntyr
Copy link
Contributor

juntyr commented Feb 21, 2025

Could you also add doc tests to confirm this behaviour?

@alex-semenyuk
Copy link
Member

Triage: comments are not addressed
@rustbot label: +S-waiting-on-author, -S-waiting-on-review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 17, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
tgross35
tgross35 reviewed Mar 18, 2025
View reviewed changes
tgross35
tgross35 reviewed Mar 18, 2025
View reviewed changes
@tgross35
Copy link
Contributor

tgross35 commented Mar 19, 2025 •
edited
Loading

LGTM but I'll run a try job to double check f128. Could you please squash?

@bors try
r? tgross35

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 19, 2025
@bors
Auto merge of rust-lang#137357 - syvb:sv/log-docs, r=<try>
6590888 
Document results of non-positive logarithms

The integer versions of logarithm functions panic on non-positive numbers. The floating point versions have different, undocumented behaviour (-inf on 0, NaN on <0). This PR documents that.

try-job: aarch64-gnu
@bors
Copy link
Contributor

bors commented Mar 19, 2025

⌛ Trying commit ecdfd07 with merge 6590888...

tgross35
tgross35 reviewed Mar 19, 2025
View reviewed changes
@rustbot rustbot assigned tgross35 and unassigned Noratrieb Mar 19, 2025
@bors
Copy link
Contributor

bors commented Mar 19, 2025

☀️ Try build successful - checks-actions
Build commit: 6590888 (6590888bf8bdbefa19a47298ffae325f55c5c15a)

@syvb
Copy link
Contributor Author

syvb commented Mar 19, 2025

I squashed up all the commits.

@tgross35
Copy link
Contributor

Looks great, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2025

📌 Commit 0199d04 has been approved by tgross35

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 19, 2025
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 21, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2025
@bors
Auto merge of rust-lang#138768 - matthiaskrgr:rollup-nfu3cm3, r=matth…
4ac032f 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137357 (Document results of non-positive logarithms)
 - rust-lang#138650 (Optimize `io::Write::write_fmt` for constant strings)
 - rust-lang#138694 (Fix: add ohos target notes)
 - rust-lang#138713 (interpret memory access hooks: also pass through the Pointer used for the access)
 - rust-lang#138724 (Check attrs: Don't try to retrieve the name of list stems)
 - rust-lang#138743 (bootstrap: add `--ci` flag)
 - rust-lang#138751 (Fix the "used_with_archive" test on Fuchsia)
 - rust-lang#138754 (Handle spans of `~const`, `const`  and `async` trait bounds in macro expansion)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1530b03 into rust-lang:master Mar 21, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 21, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 21, 2025
@rust-timer
Unrolled build for rust-lang#137357
c02eca0 
Rollup merge of rust-lang#137357 - syvb:sv/log-docs, r=tgross35

Document results of non-positive logarithms

The integer versions of logarithm functions panic on non-positive numbers. The floating point versions have different, undocumented behaviour (-inf on 0, NaN on <0). This PR documents that.

try-job: aarch64-gnu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgross35 tgross35

Assignees

@tgross35 tgross35

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.87.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@syvb @rustbot @rust-log-analyzer @tgross35 @juntyr @alex-semenyuk @bors @Noratrieb