Rename OOM to allocation error #51543

SimonSapin · 2018-06-13T22:39:23Z

The acronym is not descriptive unless one has seen it before.

Rename the oom function to handle_alloc_error. It was stabilized in 1.28, so if we do this at all we need to land it this cycle.
Rename set_oom_hook to set_alloc_error_hook
Rename take_oom_hook to take_alloc_error_hook

Bikeshed: on v.s. for, alloc v.s. allocator, error v.s. failure

rust-highfive · 2018-06-13T22:39:26Z

r? @aidanhs

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

SimonSapin · 2018-06-13T22:40:59Z

r? @sfackler

Team sign off for change to #[stable]-in-Nightly API:

@rfcbot fcp merge

Mild priority to hopefully get in this cycle:

@bors p=1

rfcbot · 2018-06-13T22:41:00Z

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

withoutboats · 2018-06-13T22:45:34Z

Is the use of the word "bail" something we've done elsewhere?

SimonSapin · 2018-06-13T22:54:16Z

@withoutboats Not that I know of. I’m open to another way to express "this this what you call when you don’t want to handle that condition". I’ve avoided "abort" because of https://github.com/rust-lang/rfcs/blob/master/text/2116-alloc-me-maybe.md#oompanic

withoutboats · 2018-06-13T23:31:08Z

The main reason I'm not excited about bail is its used in other contexts to mean something very different: for example, both error-chain and failure have bail! macros that return errors from the current function.

I'm not sure what other word would be appropriate, though..

rust-highfive · 2018-06-13T23:37:39Z

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.

/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Downloading https://files.pythonhosted.org/packages/e2/29/f5b58658602baf037db0f650567d95e8d36104e1bd1966fa4628d7c7e470/awscli-1.15.38-py2.py3-none-any.whl (1.3MB)
    0% |▎                               | 10kB 12.7MB/s eta 0:00:01
    1% |▌                               | 20kB 1.8MB/s eta 0:00:01
    2% |▉                               | 30kB 2.2MB/s eta 0:00:01
    3% |█                               | 40kB 1.9MB/s eta 0:00:01
---
travis_time:start:test_run-pass
Check compiletest suite=run-pass mode=run-pass (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:47:45] 
[00:47:45] running 3024 tests
[00:47:59] ......F.............................................................................................
[00:48:30] ....................................................................................................
[00:48:45] ....................................................................................................
[00:48:57] ....................................................................................................
[00:49:18] ....................................................................................................
---
[00:53:13] .......................................................i............................................
[00:53:33] ......................................................i.............................................
[00:54:02] ..................................................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:54:02] ..
[00:54:17] ........F........................................................F..................................
[00:55:17] ...........................................i.ii.....................................................
[00:55:47] ...................................................iiiiiii..........................................
[00:56:06] ....................................................................................................
[00:56:23] ....................................................................................................
---
[00:56:52] ---- [run-pass] run-pass/realloc-16687.rs stdout ----
[00:56:52] 
[00:56:52] error: compilation failed!
[00:56:52] status: exit code: 101
[00:56:52] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/realloc-16687.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/realloc-16687/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/realloc-16687/auxiliary"
[00:56:52] ------------------------------------------
[00:56:52] 
[00:56:52] ------------------------------------------
[00:56:52] stderr:
[00:56:52] stderr:
[00:56:52] ------------------------------------------
[00:56:52] error[E0432]: unresolved import `std::alloc::oom`
[00:56:52]   --> /checkout/src/test/run-pass/realloc-16687.rs:18:41
[00:56:52]    |
[00:56:52] 18 | use std::alloc::{Global, Alloc, Layout, oom};
[00:56:52]    |                                         ^^^ no `oom` in `alloc`
[00:56:52] error: aborting due to previous error
[00:56:52] 
[00:56:52] For more information about this error, try `rustc --explain E0432`.
[00:56:52] 
---
[00:56:52] ---- [run-pass] run-pass/regions-mock-codegen.rs stdout ----
[00:56:52] 
[00:56:52] error: compilation failed!
[00:56:52] status: exit code: 101
[00:56:52] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/regions-mock-codegen.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/regions-mock-codegen/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/regions-mock-codegen/auxiliary"
[00:56:52] ------------------------------------------
[00:56:52] 
[00:56:52] ------------------------------------------
[00:56:52] stderr:
[00:56:52] stderr:
[00:56:52] ------------------------------------------
[00:56:52] error[E0432]: unresolved import `std::alloc::oom`
[00:56:52]   --> /checkout/src/test/run-pass/regions-mock-codegen.rs:15:41
[00:56:52]    |
[00:56:52] 15 | use std::alloc::{Alloc, Global, Layout, oom};
[00:56:52]    |                                         ^^^ no `oom` in `alloc`
[00:56:52] error: aborting due to previous error
[00:56:52] 
[00:56:52] For more information about this error, try `rustc --explain E0432`.
[00:56:52] 
[00:56:52] 
[00:56:52] ------------------------------------------
[00:56:52] 
[00:56:52] thread '[run-pass] run-pass/regions-mock-codegen.rs' panickWed, 13 Jun 2018 23:37:35 GMT

The command "date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
" exited with 0.
travis_fold:start:after_failure.1

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)

Mark-Simulacrum · 2018-06-14T00:19:44Z

Note that we could backport the name change to beta, though I'd rather not. If there are concerns over naming, perhaps we should land a change de-stabilizing oom for the time being? I suppose that would be rather hard...

SimonSapin · 2018-06-14T02:16:54Z

The FCP is used as a tool to get more eyes on this, but we won’t actually wait for the 10 days period. Destabilizing is an option, but I expect we won’t need it.

mark-i-m · 2018-06-14T02:44:19Z

I don't understand what's wrong with oom. It seems like pretty standard terminology to me.

sfackler · 2018-06-14T03:01:44Z

I am not convinced this is an improvement. If there's uncertainty we should destabilize this function.

hanna-kruppe · 2018-06-14T07:56:37Z

🚲 How about report_alloc_error for oom?

glandium · 2018-06-14T12:05:04Z

handle_alloc_error?

SimonSapin · 2018-06-14T14:43:34Z

@rkruppe I think that this function is less about reporting (which might or might not happen) than it is about never returning so that callers don’t need to care about that code path anymore.

withoutboats · 2018-06-14T16:40:42Z

The language theory for this is to diverge, but I'm not sure if diverge_on_alloc_error is a great name.

mark-i-m · 2018-06-15T02:26:29Z

Sorry, could someone please explain why we need renaming at all? Oom is pretty standard terminology in systems.

SimonSapin · 2018-06-15T06:48:43Z

I think that how standard OOM sounds depends on one’s background, but more importantly these APIs are used for any kind of error/failure that is not necessarily caused but exhaustion of available memory. (For example, the requested alignment is not supported by this allocator.)

SimonSapin · 2018-06-15T20:48:14Z

To be clear: I think renaming would be nicer, but if we don’t reach consensus in time I can live with oom and I’d rather not unstabilize.

withoutboats · 2018-06-15T20:51:54Z

This reminds me of how before 1.0 all our filesystem APIs were called things like pwd instead of current_dir. Context-specific abbreviations seem strictly worse than a straightforwardly self-explanatory API label. @SimonSapin's point about how this is not strictly about oom, but any kind of allocation failure, is also compelling.

SimonSapin · 2018-06-18T13:40:46Z

handle_alloc_error is more vague but probably good enough, if we want to avoid the word "bail".

withoutboats · 2018-06-18T19:30:22Z

handle_alloc_error seems fine to me :)

The acronym is not descriptive unless one has seen it before. * Rename the `oom` function to `handle_alloc_error`. It was **stabilized in 1.28**, so if we do this at all we need to land it this cycle. * Rename `set_oom_hook` to `set_alloc_error_hook` * Rename `take_oom_hook` to `take_alloc_error_hook` Bikeshed: `alloc` v.s. `allocator`, `error` v.s. `failure`

SimonSapin · 2018-06-19T15:03:19Z

Done :)

rfcbot · 2018-06-19T15:03:22Z

🔔 This is now entering its final comment period, as per the review above. 🔔

SimonSapin · 2018-06-19T18:15:29Z

Diff is very mechanical.

@bors r+

bors · 2018-06-19T18:15:30Z

📌 Commit 2b789bd has been approved by SimonSapin

bors · 2018-06-19T19:22:19Z

⌛ Testing commit 2b789bd with merge d692ab4...

Rename OOM to allocation error The acronym is not descriptive unless one has seen it before. * Rename the `oom` function to `handle_alloc_error`. It was **stabilized in 1.28**, so if we do this at all we need to land it this cycle. * Rename `set_oom_hook` to `set_alloc_error_hook` * Rename `take_oom_hook` to `take_alloc_error_hook` Bikeshed: `on` v.s. `for`, `alloc` v.s. `allocator`, `error` v.s. `failure`

bors · 2018-06-19T21:31:31Z

☀️ Test successful - status-appveyor, status-travis
Approved by: SimonSapin
Pushing d692ab4 to master...

See rust-lang/rust#51543.

SimonSapin added A-allocators Area: Custom and system allocators T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 13, 2018

rust-highfive assigned aidanhs Jun 13, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2018

rust-highfive assigned sfackler and unassigned aidanhs Jun 13, 2018

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jun 13, 2018

SimonSapin force-pushed the oom branch from e780cce to a60103b Compare June 13, 2018 23:54

Mark-Simulacrum added the P-high High priority label Jun 14, 2018

SimonSapin mentioned this pull request Jun 14, 2018

Tracking issue for the #[alloc_error_handler] attribute (for no_std + liballoc) #51540

Open

4 tasks

SimonSapin added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jun 16, 2018

SimonSapin mentioned this pull request Jun 17, 2018

Tracking issue for the OOM hook (alloc_error_hook) #51245

Open

5 tasks

SimonSapin force-pushed the oom branch from a60103b to 2b789bd Compare June 18, 2018 19:41

rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Jun 19, 2018

rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jun 19, 2018

bors merged commit 2b789bd into rust-lang:master Jun 19, 2018

This was referenced Jun 19, 2018

Make the public API of the alloc crate a subset of std #51569

Merged

Move OOM handling to liballoc and remove the oom lang item #51607

Closed

SimonSapin deleted the oom branch June 19, 2018 22:12

rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 29, 2018

kennytm added a commit to rust-lang/nomicon that referenced this pull request Jul 7, 2018

Fix test failure, oom has been renamed to handle_alloc_error.

895897e

See rust-lang/rust#51543.

kennytm mentioned this pull request Jul 7, 2018

Fix test failure, oom has been renamed to handle_alloc_error. rust-lang/nomicon#77

Closed

apiraino removed the P-high High priority label Feb 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename OOM to allocation error #51543

Rename OOM to allocation error #51543

SimonSapin commented Jun 13, 2018 •

edited

Loading

rust-highfive commented Jun 13, 2018

SimonSapin commented Jun 13, 2018

rfcbot commented Jun 13, 2018 •

edited by sfackler

Loading

withoutboats commented Jun 13, 2018

SimonSapin commented Jun 13, 2018

withoutboats commented Jun 13, 2018

rust-highfive commented Jun 13, 2018

Mark-Simulacrum commented Jun 14, 2018

SimonSapin commented Jun 14, 2018

mark-i-m commented Jun 14, 2018

sfackler commented Jun 14, 2018

hanna-kruppe commented Jun 14, 2018

glandium commented Jun 14, 2018

SimonSapin commented Jun 14, 2018

withoutboats commented Jun 14, 2018 •

edited

Loading

mark-i-m commented Jun 15, 2018

SimonSapin commented Jun 15, 2018

SimonSapin commented Jun 15, 2018

withoutboats commented Jun 15, 2018

SimonSapin commented Jun 18, 2018

withoutboats commented Jun 18, 2018

SimonSapin commented Jun 19, 2018

rfcbot commented Jun 19, 2018

SimonSapin commented Jun 19, 2018

bors commented Jun 19, 2018

bors commented Jun 19, 2018

bors commented Jun 19, 2018

Rename OOM to allocation error #51543

Rename OOM to allocation error #51543

Conversation

SimonSapin commented Jun 13, 2018 • edited Loading

rust-highfive commented Jun 13, 2018

SimonSapin commented Jun 13, 2018

rfcbot commented Jun 13, 2018 • edited by sfackler Loading

withoutboats commented Jun 13, 2018

SimonSapin commented Jun 13, 2018

withoutboats commented Jun 13, 2018

rust-highfive commented Jun 13, 2018

Mark-Simulacrum commented Jun 14, 2018

SimonSapin commented Jun 14, 2018

mark-i-m commented Jun 14, 2018

sfackler commented Jun 14, 2018

hanna-kruppe commented Jun 14, 2018

glandium commented Jun 14, 2018

SimonSapin commented Jun 14, 2018

withoutboats commented Jun 14, 2018 • edited Loading

mark-i-m commented Jun 15, 2018

SimonSapin commented Jun 15, 2018

SimonSapin commented Jun 15, 2018

withoutboats commented Jun 15, 2018

SimonSapin commented Jun 18, 2018

withoutboats commented Jun 18, 2018

SimonSapin commented Jun 19, 2018

rfcbot commented Jun 19, 2018

SimonSapin commented Jun 19, 2018

bors commented Jun 19, 2018

bors commented Jun 19, 2018

bors commented Jun 19, 2018

SimonSapin commented Jun 13, 2018 •

edited

Loading

rfcbot commented Jun 13, 2018 •

edited by sfackler

Loading

withoutboats commented Jun 14, 2018 •

edited

Loading