sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land.
================ Comment at: clang-tools-extra/clangd/Protocol.cpp:425 +const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_TWEAK = + "clangd.applyCodeAction"; + ---------------- tweak or applyTweak ================ Comment at: clang-tools-extra/clangd/Protocol.h:634 +struct TweakArgs { + URIForFile file; ---------------- comment here? ================ Comment at: clang-tools-extra/clangd/SourceCode.h:59 +/// care to avoid comparing the result with expansion locations. +llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM, + Position P); ---------------- (can we add a unit test for this function?) ================ Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:27 +std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S) { +#ifndef NDEBUG + { ---------------- ilya-biryukov wrote: > Please note I added these assertions here. > > It feels weird to traverse twice on every call to `prepareTweaks`, but that's > the simplest option I could come up with. Looks fine - you could consider extracting to a separate function `validateRegistry` or so, but up to you CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56267/new/ https://reviews.llvm.org/D56267 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits