daiyousei-qz added inline comments.

================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:616
+    Position HintStart = sourceLocToPosition(SM, BraceRange.getEnd());
+    Position HintEnd = {HintStart.line,
+                        HintStart.character +
----------------
sammccall wrote:
> please don't do this, call sourceLocToPosition unless you have benchmarks 
> showing this is a bottleneck on real-world code.
> 
> `lspLength` and direct manipulation of line/character is unneccesarily subtle.
> (If the performance matters, there's no need to be computing the LSP line of 
> the lbrace at all - we never use it - this is one reason I think this 
> function has the wrong signature)
I argue performance does matter here. I eye-balled inlay hint compute time in 
clangd's output window from vscode. On "./clang/lib/Sema/SemaOpenMP.cpp" which 
is around 1MB, using `sourceLocToPosition` roughly takes around 1250~1350ms. 
However, using two `offsetLocToPosition` usually use 1400~1500ms.  I think 
100ms delta is already a noticeable difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150635

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to