Rename OOM to allocation error

[SimonSapin]

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]

r? @aidanhs

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

[SimonSapin]

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]

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

• @Kimundi
• @SimonSapin
• @alexcrichton
• @dtolnay
• @sfackler
• @withoutboats

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]

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

[SimonSapin]

@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]

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]

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]

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]

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]

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

[sfackler]

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

[hanna-kruppe]

🚲 How about report_alloc_error for oom?

[glandium]

handle_alloc_error?

[SimonSapin]

@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]

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]

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

[SimonSapin]

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]

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]

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]

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

[withoutboats]

handle_alloc_error seems fine to me :)

[SimonSapin]

Done :)

[rfcbot]

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

[SimonSapin]

Diff is very mechanical.

@bors r+

[bors]

📌 Commit 2b789bd has been approved by SimonSapin

[bors]

⌛ Testing commit 2b789bd with merge d692ab4...

[bors]

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