[PATCH] D51033: [DWARF] Move error printing from DWARF classes to Support library. NFC.

Victor Leschuk via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 21 05:50:11 PDT 2018


vleschuk added inline comments.


================
Comment at: include/llvm/Support/Error.h:912
 
+/// Display error with color
+void DisplayError(Error Error);
----------------
jhenderson wrote:
> Comments should have trailing full stops.
> 
> I don't understand the phrase here. Better would be to say something like "Print an Error as an error message on stderr". I think the use of WithColor should be irrelevant to the function description.
Will do.


================
Comment at: include/llvm/Support/Error.h:913
+/// Display error with color
+void DisplayError(Error Error);
+
----------------
jhenderson wrote:
> These should use lower case first for the first letter (i.e. `displayError` etc).
Yeah, let's use dumpError().


================
Comment at: lib/DebugInfo/DWARF/DWARFDebugAddr.cpp:22-23
 
-Error DWARFDebugAddrTable::extract(DWARFDataExtractor Data,
-                                   uint32_t *OffsetPtr,
-                                   uint16_t Version,
-                                   uint8_t AddrSize,
+Error DWARFDebugAddrTable::extract(DWARFDataExtractor Data, uint32_t *OffsetPtr,
+                                   uint16_t Version, uint8_t AddrSize,
                                    std::function<void(Error)> WarnCallback) {
----------------
jhenderson wrote:
> Unrelated whitespace change should not be in this commit.
Will revert.


Repository:
  rL LLVM

https://reviews.llvm.org/D51033





More information about the llvm-commits mailing list