[PATCH] D51867: [Diagnostics][NFC] Add error handling to FormatDiagnostic()

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Regarding the asserts to catch potential problems, seems most of them are for buffer overflows. Aren't sanitizers catching those cases, specifically Address Sanitizer? I haven't checked, just seems it would be good to check buffer overflow automatically instead of using explicit asserts.

Also there are a few changes I wouldn't call NFC. Those change loop iteration from "iterator != end" to "iterator < end". As it is functionality change, I'd like to have tests to cover that. Also I've fixed a few bugs with going past the end of buffer and bugs were actually inside the loop, not with buffer range check. It is tempting to play safe but it has a risk of hiding real bugs.



================
Comment at: lib/Basic/Diagnostic.cpp:804
+
+  while (DiagStr < DiagEnd) {
     if (DiagStr[0] != '%') {
----------------
For example, I wouldn't call this one NFC.


Repository:
  rC Clang

https://reviews.llvm.org/D51867