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

2020-09-14 Thread Sam McCall via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG30667c967d3f: [clangd] Add error() function for creating formatv-style llvm::Errors. NFC (authored by sammccall). Changed prior to commit: https:/

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

2020-07-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. anyway, I'm fine with this change, feel free to submit it as-is (or do any refinements), don't really want the naming to block this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

2020-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83419#2141433 , @hokein wrote: > > Sorry, I should give some reasons here: > > These are sensible reasons. My only (not a strong) concern is that "error" is > quite special, we need to be careful to choose it -- note that th

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

2020-07-09 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. > Sorry, I should give some reasons here: These are sensible reasons. My only (not a strong) concern is that "error" is quite special, we need to be careful to choose it -- note that there is an `error` function in glibc which is used for error-reporting. maybe there ar

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

2020-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D83419#2140889 , @sammccall wrote: > Honestly, I really dislike names in the style of makeError/createError. Sorry, I should give some reasons here: - not discoverable (`error` is marginally better) - emphasis on allocation

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

2020-07-09 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. Honestly, I really dislike names in the style of makeError/createError. My favorite name for this would be Error (i.e. a constructor) but that would involve redesigning it (which is a good idea, but an impossible yak-shave). I guess makeError is still better than the st

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

2020-07-08 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment. In D83419#2140573 , @sammccall wrote: > In D83419#2140311 , @njames93 wrote: > > > I'm not a huge fan of using `error`, maybe `createError`, > > Its definitely out of place in Logger, but t

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

2020-07-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In 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 overk

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

2020-07-08 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. I'm not a huge fan of using `error`, maybe `createError`, its used in many other parts of llvm, typically with static linkage. 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. Repository:

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

2020-07-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 276531. sammccall added a comment. Move a few comments around. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83419/new/ https://reviews.llvm.org/D83419 Files: clang-tools-extra/clangd/ClangdLSPServer.cpp

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

2020-07-08 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision. sammccall added reviewers: kbobyrev, hokein. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, javed.absar, ilya-biryukov, mgorny. Herald added a project: clang. This is considerably terser than the makeStringError and friends