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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
