[PATCH] D79745: [YAMLTraits] Add trait for char

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

I don't believe any solution based on conditional compilation (ifdefs, feature detection, etc.) would help here. The reason for that is that it's impossible to assign consistent semantics to `char` and `uint8_t` everywhere.

- Currently, the "desired" semantics for `uint8_t` is to treat it as a number. In fact the implementation contains already contains a small hack to achieve it:

  void ScalarTraits<uint8_t>::output(const uint8_t &Val, void *,
                                     raw_ostream &Out) {
    // use temp uin32_t because ostream thinks uint8_t is a character
    uint32_t Num = Val;
    Out << Num;
  }

- The proposed desired semantics for `char` is to treat it as a character.

Both of these semantics are "reasonable", but (as we've seen) they are not actually implementable in C(++). Conditionally compiling ScalarTraits<char> will make things build on solaris, but it will mean that the yaml serialization of `char` variables will be different there. That does not really matter for the current lldb use case, but generally speaking, it's not a good situation to be in.

For the time being, I think it's perfectly reasonable to work around this problem in lldb. If you don't like reinterpret_casts (which are fairly safe in this case, as one of the types is char), you can use a solution like:

  uint8_t TheInt = TheChar;
  IO.mapRequired("whatever", TheInt);
  TheChar = TheInt;

If we wanted to define a global yaml serialization for `char`, then I think the first step should be to change the existing specializations to be defined in terms of primitive types (`(un)signed (char|short|int|...)` and `char`), because mixing primitive types and the stdint typedefs just doesn't work. Then we can discuss the appropriate serialization for each primitive type. I think it would be reasonable if all flavours of `char` printed out as a character, as it would e.g. match what raw_ostream does (whose `operator<<`s are also defined in terms of fundamental types), but I'm not sure how many dependencies we have on the current serialization of (u)int8_t (though given that the yaml code is mainly used for tests, I wouldn't expect any of those dependencies to be too critical)...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79745