[PATCH] D51860: [clangd] NFC: Use uint32_t for FuzzyFindRequest limits

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 11 02:28:54 PDT 2018


sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/index/Index.h:440
   /// return more than this, e.g. if it doesn't know which candidates are best.
-  size_t MaxCandidateCount = std::numeric_limits<size_t>::max();
+  uint32_t MaxCandidateCount = std::numeric_limits<uint32_t>::max();
   /// If set to true, only symbols for completion support will be considered.
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > kbobyrev wrote:
> > > ioeric wrote:
> > > > Or use `unsigned`?
> > > `unsigned` would have different size on different platforms, I'm not really sure we want that; could you elaborate on why you think that would be better?
> > I thought it's (almost) always 4 bytes? But it should always have a smaller size than `uint64_t` in json serialization, so it should work for us. In general, I would prefer `unsigned` to `uint32_t` when possible. For most of the platforms, they are the same. But up to you :) I don't really feel strong about this.
> BTW, many people think using unsigned ints just because inputs can't be negative is a bad idea.
> See https://stackoverflow.com/a/18796234
I mostly agree with that, but LLVM uses unsigned types pervasively, and -Wsign-compare, so they're hard to avoid.

(FWIW, I still think that this case has become complicated enough that we should use the most explicit option, which seems like Optional here)


https://reviews.llvm.org/D51860





More information about the cfe-commits mailing list