[PATCH] D28081: Make GetStyle return Expected<FormatStyle> instead of FormatStyle

Antonio Maiorano via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Jan 14 22:08:34 PST 2017


amaiorano added inline comments.


================
Comment at: lib/Format/Format.cpp:424
 
+llvm::Error make_string_error(const llvm::Twine &Message) {
+  return llvm::make_error<llvm::StringError>(Message,
----------------
alexshap wrote:
> amaiorano wrote:
> > ioeric wrote:
> > > Maybe make this `inline`?
> > Yes.
> regarding this function and some other helper functions in this file: 
> 1. why aren't they static ? 
> (if this stuff is not used outside of this .cpp -  but yeah, i assume i'm missing smth)
> 2. http://llvm.org/docs/CodingStandards.html naming conventions are
> "Function names should be verb phrases (as they represent actions), and command-like function should be imperative. The name should be camel case, and start with a lower case letter (e.g. openFile() or isFoo())"  
1. IMHO I think inline is a valid substitute for static. Even though inline functions have external linkage by default, in practice they get inlined and not seen by the linker, or handled properly by the linker (it picks one).

2. You're right, but the reason I went with make_error_code was because I'm wrapping a call to llvm::make_error, which already breaks the camel case convention. But I have no problem renaming this makeErrorCode, just let me know.



https://reviews.llvm.org/D28081





More information about the cfe-commits mailing list