[PATCH] D40089: [clangd] Make completion scores use 0-1 floats internally.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

A few comments based on our discussion.



================
Comment at: clangd/ClangdUnit.cpp:387
 
   std::string sortText() const {
     std::string S, NameStorage;
----------------
example


================
Comment at: clangd/ClangdUnit.cpp:416
+
+  // Produces an integer that sorts in the same order as F.
+  static uint32_t encodeFloat(float F) {
----------------
nit: mention that if a < b, encodeFloat(a) < encodeFloat(b)


================
Comment at: clangd/ClangdUnit.cpp:417
+  // Produces an integer that sorts in the same order as F.
+  static uint32_t encodeFloat(float F) {
+    // IEEE 754 floats compare like 2s complement integers.
----------------
I think we'd better have unittest for this function.


================
Comment at: clangd/ClangdUnit.cpp:423
+    memcpy(&U, &F, sizeof(float));
+    // Convert U from 2s complement to unsigned, preserving order.
+    const uint32_t SignBit = ~(~uint32_t{0} >> 1);
----------------
Would be nice to explain more what we do here: we are trying to remap from the signed range [INT_MIN, INT_MAX] to the unsigned range [0, UINT_MAX], so that we can compare them via string literal comparison.


================
Comment at: clangd/ClangdUnit.cpp:425
+    const uint32_t SignBit = ~(~uint32_t{0} >> 1);
+    return U & SignBit ? 0 - U : U + SignBit;
+  }
----------------
U ^ SignBit


================
Comment at: test/clangd/completion-items-kinds.test:1
 # RUN: clangd -enable-snippets -run-synchronously < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
----------------
an off-topic comment: why not using `-pretty` in this test?


https://reviews.llvm.org/D40089