[PATCH] D83419: [clangd] Add error() function for creating formatv-style llvm::Errors. NFC

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D83419#2140573 <https://reviews.llvm.org/D83419#2140573>, @sammccall wrote:

> In D83419#2140311 <https://reviews.llvm.org/D83419#2140311>, @njames93 wrote:
>
> > I'm not a huge fan of using `error`, maybe `createError`, 
> >  Its definitely out of place in Logger, but there is no where else better for it to live short of a new file which seems overkill.
>
>
> Yeah, the naming is definitely the hardest part to be sure of here.
>  It's an extremely commonly used function: 75 references to StringError + another 20 or so references to local helpers.
>  This puts it in the same class as log (95), vlog (50), elog(85).
>  It's also similar in that it is cross cutting and has the same format string + args shape (so wrapping affects it similarly).


while the name `error` aligns well with `elog/vlog`, I'm not in favor of it. I think the key point is that `error` creates an **`Error`** object, and this object must be checked before destructing. `error` name makes it look like a print function, I'd prefer to add a verb to the name,  e.g. `makeError`

>> its used in many other parts of llvm, typically with static linkage.
> 
> Yeah, there's some precedent, though not overwhelming consensus. Outside clangd I count 10 `createError`s, 7 `make_string_error`s, 4 `error`s, 1 `makeError`, 1 `makeStringError`, and the canonical `makeStringError` and `make_error`.
> 
> But I think this is just a place where wider LLVM is making different tradeoffs:
> 
> - for good reasons:  many errors are diagnostics or don't need to be carefully routed back to a specific request
> - for bad reasons: e.g. compliance with the "verb phrase" policy that's poorly observed and itself seems like a sad accident of history: http://lists.llvm.org/pipermail/llvm-dev/2018-May/thread.html#123547
> - llvm::Error itself seems like a pretty strong argument that existing practice doesn't guarantee good ergonomics

it would be nicer if we could offer this across llvm-projects, but agree that logger.cc seems to be the best place to place it so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83419