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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I'm not convinced "Display" is the right term for the message. "Print" or "Dump" would be much more typical for reporting messages.



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:252
                 const DWARFContext &Ctx, const DWARFUnit *U,
-                std::function<void(Error)> RecoverableErrorCallback = warn,
+                std::function<void(Error)> RecoverableErrorCallback = DisplayWarning,
                 raw_ostream *OS = nullptr);
----------------
vleschuk wrote:
> I know that this exceeds 80 chars per line, but that's what clang-format suggested.
clang-format?


================
Comment at: include/llvm/Support/Error.h:912
 
+/// Display error with color
+void DisplayError(Error Error);
----------------
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.


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


================
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) {
----------------
Unrelated whitespace change should not be in this commit.


Repository:
  rL LLVM

https://reviews.llvm.org/D51033