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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVector.h:276
+                         std::to_string(this->SizeTypeMax()) + ")";
+#ifdef LLVM_ENABLE_EXCEPTIONS
+    throw std::length_error(Reason);
----------------
If you want to use `#ifdef LLVM_ENABLE_EXCEPTIONS`

```
#ifdef LLVM_ENABLE_EXCEPTIONS
#else
#endif
```


================
Comment at: llvm/lib/Support/ErrorHandling.cpp:175
+  (void)::write(2, Reason, strlen(Reason));
+  (void)::write(2, newline, strlen(newline));
   abort();
----------------
GMNGeoffrey wrote:
> MaskRay wrote:
> > `(void)::write(2, "\n", 1);`
> The way I had it seems more explicit about what's going on here (same reason OOMMessage is a separate variable, I assume). It's obviously easier to count bytes in newline, but seems better to let the compiler do it. I figured the compiler should optimize it to the same thing anyway, but fiddling with godbolt, it seems that's not actually the case. The `char[]` is still allocated. This actually applies to the `OOMMessage` variable too. Switching it to `const char*` (or `auto`, which deduces this) makes the compiled instructions the same, and the source code more explicit, I think. https://godbolt.org/z/q5KzhK
`const char OOMMessage[] = ...`

* clang performs constant evaluation of strlen while emitting LLVM IR.

`const char *OOMMessage = ...`

* clang does not evaluate the length.
* SimplifyLibCalls.cpp called by instcombine can perform constant evaluation of strlen.

`char OOMMessage[] = ...`

* clang does evaluate the length.
* SimplifyLibCalls.cpp called by instcombine cannot optimize the call.
* There is a runtime string copy and a runtime call of strlen.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86892