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

Make integer exponentiation methods unstably const #68978

Merged
merged 4 commits into from Feb 20, 2020

Conversation

@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Feb 9, 2020

cc #53718

This makes the following inherent methods on integer primitives into unstable const fn:

  • pow
  • checked_pow
  • wrapping_pow
  • overflowing_pow
  • saturating_pow
  • next_power_of_two
  • checked_next_power_of_two
  • wrapping_next_power_of_two

Only two changes were made to the implementation of these methods. First, I had to switch from the ? operator, which is not yet implemented in a const context, to a try_opt macro. Second, next_power_of_two was using ops::Add::add (see the first commit) to "get overflow checks", so I switched to #[rustc_inherit_overflow_checks]. I'm not quite sure why the attribute wasn't used in the first place.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Feb 9, 2020

r? @cramertj

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

@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:const-int-pow branch from 69779b5 to 4ce006b Feb 9, 2020
I believe the previous code was calling `ops::Add::add` instead of the
`+` operator to get this behavior.
@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:const-int-pow branch from 4ce006b to eadd0cf Feb 9, 2020
@rust-highfive

This comment was marked as outdated.

Copy link
Collaborator

rust-highfive commented Feb 9, 2020

The job x86_64-gnu-llvm-7 of your PR failed (pretty log, 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.
2020-02-09T01:20:12.0537970Z ========================== Starting Command Output ===========================
2020-02-09T01:20:12.0540805Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/6c02595e-3309-4f49-88c6-0b99cf8f90fb.sh
2020-02-09T01:20:12.0540848Z 
2020-02-09T01:20:12.0543827Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-09T01:20:12.0549820Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68978/merge to s
2020-02-09T01:20:12.0551662Z Task         : Get sources
2020-02-09T01:20:12.0551696Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-09T01:20:12.0551774Z Version      : 1.0.0
2020-02-09T01:20:12.0551807Z Author       : Microsoft
---
2020-02-09T01:20:13.0230992Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-09T01:20:13.0243908Z ##[command]git config gc.auto 0
2020-02-09T01:20:13.0248462Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-09T01:20:13.0252788Z ##[command]git config --get-all http.proxy
2020-02-09T01:20:13.0266048Z ##[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/68978/merge:refs/remotes/pull/68978/merge
---
2020-02-09T02:19:50.3375761Z .................................................................................................... 1700/9612
2020-02-09T02:19:55.3399796Z .................................................................................................... 1800/9612
2020-02-09T02:20:07.3685506Z ...............................i.................................................................... 1900/9612
2020-02-09T02:20:15.1331024Z .................................................................................................... 2000/9612
2020-02-09T02:20:29.5432305Z .....................iiiii.......................................................................... 2100/9612
2020-02-09T02:20:39.4238566Z .................................................................................................... 2300/9612
2020-02-09T02:20:41.8811730Z .................................................................................................... 2400/9612
2020-02-09T02:20:46.6070324Z .................................................................................................... 2500/9612
2020-02-09T02:21:07.7941054Z .................................................................................................... 2600/9612
---
2020-02-09T02:23:45.2048631Z .........................................................................i...............i.......... 4900/9612
2020-02-09T02:23:52.8331018Z .................................................................................................... 5000/9612
2020-02-09T02:24:01.1239913Z .................................................................................................... 5100/9612
2020-02-09T02:24:05.8346027Z ................i................................................................................... 5200/9612
2020-02-09T02:24:16.6434856Z ..........................................................................................ii.ii..... 5300/9612
2020-02-09T02:24:21.0221422Z ...i...i............................................................................................ 5400/9612
2020-02-09T02:24:31.0156890Z .................................................................................................... 5600/9612
2020-02-09T02:24:41.5565641Z ..............................................................................i..................... 5700/9612
2020-02-09T02:24:49.1779180Z .................................................................................................... 5800/9612
2020-02-09T02:24:55.4173550Z .................................................................................................... 5900/9612
2020-02-09T02:24:55.4173550Z .................................................................................................... 5900/9612
2020-02-09T02:25:05.6454704Z ......................................................................ii...i..ii...........i........ 6000/9612
2020-02-09T02:25:27.3069373Z .................................................................................................... 6200/9612
2020-02-09T02:25:34.9644301Z .................................................................................................... 6300/9612
2020-02-09T02:25:40.6391771Z ..................................................................................................i. 6400/9612
2020-02-09T02:25:40.6391771Z ..................................................................................................i. 6400/9612
2020-02-09T02:25:53.9203657Z .ii................................................................................................. 6500/9612
2020-02-09T02:26:13.7878293Z .....................................................................................i.............. 6700/9612
2020-02-09T02:26:15.9515993Z .................................................................................................... 6800/9612
2020-02-09T02:26:18.1507871Z ............................................................................................i....... 6900/9612
2020-02-09T02:26:20.8186044Z .................................................................................................... 7000/9612
---
2020-02-09T02:27:58.7631464Z .................................................................................................... 7600/9612
2020-02-09T02:28:03.6751329Z .................................................................................................... 7700/9612
2020-02-09T02:28:08.8955109Z .................................................................................................... 7800/9612
2020-02-09T02:28:18.1357512Z .................................................................................................... 7900/9612
2020-02-09T02:28:27.1922201Z ........................................................iiiiiii.i................................... 8000/9612
2020-02-09T02:28:42.6004608Z ...i................................................................................................ 8200/9612
2020-02-09T02:28:48.2963792Z .................................................................................................... 8300/9612
2020-02-09T02:29:04.1016372Z .................................................................................................... 8400/9612
2020-02-09T02:29:13.0132882Z .................................................................................................... 8500/9612
---
2020-02-09T02:31:15.4263953Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:348:22
2020-02-09T02:31:15.4264071Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2020-02-09T02:31:15.4276930Z 
2020-02-09T02:31:15.4277195Z 
2020-02-09T02:31:15.4282623Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-7/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "7.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2020-02-09T02:31:15.4283467Z 
2020-02-09T02:31:15.4283686Z 
2020-02-09T02:31:15.4294151Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2020-02-09T02:31:15.4294435Z Build completed unsuccessfully in 1:04:46
2020-02-09T02:31:15.4294435Z Build completed unsuccessfully in 1:04:46
2020-02-09T02:31:15.4348760Z == clock drift check ==
2020-02-09T02:31:15.4364122Z   local time: Sun Feb  9 02:31:15 UTC 2020
2020-02-09T02:31:15.7081706Z   network time: Sun, 09 Feb 2020 02:31:15 GMT
2020-02-09T02:31:15.7081816Z == end clock drift check ==
2020-02-09T02:31:16.1112760Z 
2020-02-09T02:31:16.1215849Z ##[error]Bash exited with code '1'.
2020-02-09T02:31:16.1228533Z ##[section]Finishing: Run build
2020-02-09T02:31:16.1251696Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68978/merge to s
2020-02-09T02:31:16.1254084Z Task         : Get sources
2020-02-09T02:31:16.1254133Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-09T02:31:16.1254199Z Version      : 1.0.0
2020-02-09T02:31:16.1254244Z Author       : Microsoft
2020-02-09T02:31:16.1254244Z Author       : Microsoft
2020-02-09T02:31:16.1254291Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-09T02:31:16.1254343Z ==============================================================================
2020-02-09T02:31:16.5709841Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-09T02:31:16.5787972Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/68978/merge to s
2020-02-09T02:31:16.5929660Z Cleaning up task key
2020-02-09T02:31:16.5930459Z Start cleaning up orphan processes.
2020-02-09T02:31:16.6103104Z Terminate orphan process: pid (11456) (python)
2020-02-09T02:31:16.6333242Z ##[section]Finishing: Finalize Job

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)

@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:const-int-pow branch from eadd0cf to 7fe5eaf Feb 9, 2020
@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

ecstatic-morse commented Feb 9, 2020

So I found #45754, which originally used #[rustc_inherit_overflow_checks] for next_power_of_two but switched to ops::Add::add. Is there a rule against adding that attribute in new places? The test added in that PR passes with my changes. cc @scottmcm.

@ecstatic-morse ecstatic-morse changed the title Make integer exponentiation methods const Make integer exponentiation methods unstably const Feb 9, 2020
@Centril Centril added the const-hack label Feb 10, 2020
@Centril

This comment has been minimized.

Copy link
Member

Centril commented Feb 10, 2020

@oli-obk

This comment has been minimized.

Copy link
Contributor

oli-obk commented Feb 10, 2020

lgtm, we should remove the macro at some point, but that's not really a problem to leave around until someone discovers it.

I also don't know about the panic situation in that function. r=me with @scottmcm 's approval on that panic "change"

@scottmcm

This comment has been minimized.

Copy link
Member

scottmcm commented Feb 20, 2020

I have no horse in this race; I added it because I was asked to do so by @kennytm in #45754 (comment)

AFAIK that's just a matter of "use fewer #[rustc*] things where possible", so if it's not possible in this case and compiler people are fine with it, I'm fine with it.

So I think that means
@bors r=oli-obk

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 20, 2020

📌 Commit 7fe5eaf has been approved by oli-obk

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 20, 2020
…-obk

Make integer exponentiation methods unstably const

cc rust-lang#53718

This makes the following inherent methods on integer primitives into unstable `const fn`:
- `pow`
- `checked_pow`
- `wrapping_pow`
- `overflowing_pow`
- `saturating_pow`
- `next_power_of_two`
- `checked_next_power_of_two`
- `wrapping_next_power_of_two`

Only two changes were made to the implementation of these methods. First, I had to switch from the `?` operator, which is not yet implemented in a const context, to a `try_opt` macro. Second, `next_power_of_two` was using `ops::Add::add` (see the first commit) to "get overflow checks", so I switched to `#[rustc_inherit_overflow_checks]`. I'm not quite sure why the attribute wasn't used in the first place.
bors added a commit that referenced this pull request Feb 20, 2020
Rollup of 5 pull requests

Successful merges:

 - #68705 (Add LinkedList::remove())
 - #68945 (Stabilize Once::is_completed)
 - #68978 (Make integer exponentiation methods unstably const)
 - #69266 (Fix race condition when allocating source files in SourceMap)
 - #69287 (Clean up E0317 explanation)

Failed merges:

r? @ghost
@bors bors merged commit d96951f into rust-lang:master Feb 20, 2020
4 checks passed
4 checks passed
pr Build #20200209.10 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
@TimDiekmann TimDiekmann mentioned this pull request Feb 21, 2020
59 of 59 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.