[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D43230#1006498, @ioeric wrote:

> I think it would probably depend on whether we could find a sensible default value for a field (e.g. is 0 a better default value than UINT_MAX for line number?) and whether we expect users to always initialize a field (e.g. would it be better to pollute the field instead of providing a default value so that uninitialized values would be caught more easily).


Yeah, it arguable whether 0 is the right default, but at least it's consistent with what compiler uses for zero-initialization, i.e. when you default-initialize builtin types explicitly (e.g. `int b = int();` or `int b = *(new int)`).
For things like `FileChangeType` we choose `1`, because it's the lowest acceptable value, but I'm not sure that's the right default value, merely a random valid enum value.



================
Comment at: clangd/Protocol.h:80
 struct Position {
+  Position() = default;
+  Position(int line, int character) : line(line), character(character) {}
----------------
sammccall wrote:
> I'd lean to making callers initialize field-by-field here rather than provide constructors.
> It's not terrible here (you can get it wrong, but only one way), but it's a pattern that doesn't scale so well I think.
As discussed offline, removed the ctor from Position and initialized everything on per-field basis.

It strikes me that this should be decided on per-struct basis. For things like position per-field init seems preferable to avoid accidentally confusing lines and chars.
On the other hand, for `Range` it seems totally fine to have a constructor, it's almost impossible to write `Range{end, begin}`, the ordering of ctor params is very natural.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43230