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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 8 16:57:55 PDT 2020


sammccall added a comment.

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).

> 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83419





More information about the cfe-commits mailing list