[PATCH] D139298: WIP: Add error handling to demangle API

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Adding `std::error_code` looks good.

> This is an attempt to implement that using std::optional and std::error_code

I don't see `std::optional` in the patch :)



================
Comment at: llvm/include/llvm/Demangle/Demangle.h:53
+    case llvm::DemangleError::Success:
+      return "success.";
+    }
----------------
thieta wrote:
> thieta wrote:
> > jhenderson wrote:
> > > Nit: capitalization here is inconsistent.
> > > 
> > > FWIW, the LLVM coding standards state for warning and error messages to a) start with a lower case letter, and b) not to end with a period. Whether that applies in this context is debatable.
> > Hmm seems like it should apply here, I will make it consistent.
> I missed this - but will fix in the next revision of the patch.
If there isn't a precedent, as mentioned, no-capitalization is preferred (https://llvm.org/docs/CodingStandards.html#error-and-warning-messages).

Also, no-capitalization makes the message easily composable with a function that appends a prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139298