sammccall accepted this revision.
sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:947
+ Out->tokenModifiers = Tok.Modifiers;
+ Last = Tok;
----------------
this copy feels gratuitous, can we just use Tokens.back(), or do the copy in
the rare case?
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:954
+ // line.
+ // This is slow, but code rarely has multiline tokens.
+ // FIXME: There's a client capability for supporting multiline tokens,
----------------
This isn't literally true - files with multiline raw strings aren't rare. maybe
"multiline tokens are rare"?
(even so I wonder if there's a real performance problem here...)
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:960
+ // length.
+ for (int I = Tok.R.start.line; I != Tok.R.end.line; ++I) {
+ auto LineEnd = Code.find('\n', TokStartOffset);
----------------
can we use < rather than != here? (or an assert)
I realize something has to have gone horribly wrong to infloop here, but it's
something nonlocal...
================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:971
+ Out->deltaStart = 0;
+ Out->tokenType = static_cast<unsigned>(Tok.Kind);
+ Out->tokenModifiers = Tok.Modifiers;
----------------
reassigning fields one by one seems fragile, copy the last token and then
overwrite the fields needed?
================
Comment at:
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:1007
+ llvm::StringRef AnnotatedCode = R"cpp(
+[[fo
+o
----------------
please have the first token start somewhere other than column 0
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127856/new/
https://reviews.llvm.org/D127856
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
