njames93 added inline comments.
================
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,
----------------
kadircet wrote:
> nit: I would rather make `!Invalid` a condition and just make use of it in
> here as well.
Good point, that's much nicer.
================
Comment at: clang-tools-extra/clangd/SourceCode.cpp:555
+ auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid);
+ if (!Invalid)
+ Result.newText = Insert.str();
----------------
kadircet wrote:
> 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.
While I agree with this in principal. We currently don't handle the case where
RemoveRange doesn't correspond to a FileCharRange which would likely need
propagating. Though likely in a separate patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97123/new/
https://reviews.llvm.org/D97123
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
