[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 22 08:47:35 PST 2021


kadircet added a comment.

mostly LG, can you also add some tests?



================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:695
+      if (Insert.empty() && FixIt.InsertFromRange.isValid()) {
+        bool InvalidInsert = false;
+        Insert = Lexer::getSourceText(FixIt.InsertFromRange, SM, *LangOpts,
----------------
nit: I would rather make `!Invalid` a condition and just make use of it in here as well.


================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:555
+    auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid);
+    if (!Invalid)
+      Result.newText = Insert.str();
----------------
it feels like correct semantics would be to make this function fail now. as the resulting value will likely be used for editing the source code and we don't want to propagate some garbage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97123



More information about the cfe-commits mailing list