[PATCH] D86892: Improve error handling for SmallVector programming errors.

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey added a comment.

In D86892#2248125 <https://reviews.llvm.org/D86892#2248125>, @mehdi_amini wrote:

>> I disagree. report_bad_alloc_error reports an OOM, which this is not. There is no amount of system memory that would make this not an error.
>
> What would `new` do?

What's the question exactly?

`new llvm::SmallVector<int, 4>(-1);` will also call `report_fatal_error` when it tries to grow. Note that I also tested this out with a `SmallVector` of bools, which uses `unit64_t` as the size type because of https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/ADT/SmallVector.h#L87-L89. Then `reserve(-1)` doesn't trigger this failure (because that is theoretically a size you can request), but I do get a bad_alloc_error (because I don't have that much RAM).

  LLVM ERROR: out of memory
  Allocation failed



================
Comment at: llvm/lib/Support/SmallVector.cpp:53
+    report_fatal_error("SmallVector unable to grow. Requested capacity (" +
+                       std::to_string(MinCapacity) +
+                       ") is larger than maximum capacity for size type (" +
----------------
mehdi_amini wrote:
> This use to be banned from LLVM because some stdlib we supported were buggy with this (Android maybe?)
> 
> In general I use instead Twine.
I just noticed that the normal SmallVector::grow has the same problem as this, so going to update that as well. That lives in SmallVector.h though, and trying to include Twine there gets us into all sorts of weird trouble. Is to_string still banned? Do you have another suggestion?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86892/new/

https://reviews.llvm.org/D86892