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

[Rainer Orth via Phabricator via llvm-commits]

ro added a comment.

In D79745#2049359 <https://reviews.llvm.org/D79745#2049359>, @JDevlieghere wrote:

> In D79745#2049287 <https://reviews.llvm.org/D79745#2049287>, @ro wrote:
>
> > In D79745#2049234 <https://reviews.llvm.org/D79745#2049234>, @JDevlieghere wrote:
> >
> > >
> >
>




>> I've checked the gcc configs and of those, only on Solaris and BPF is `int8_t` plain `char`.  Elsewhere they use explicit `signed char` or
>>  `unsigned char`.  Haven't checked what the C standard requires/allows here, but AFAIK Sun had a couple of guys on the C committee at
>>  the time, so they should know what they're doing...
> 
> The C++ standard [1] says that `char`, `unsigned char` and `signed char` are distinctive types. However, I doesn't specify how `uint8_t` is implemented. I think maybe the right thing to do is wrap the YAML trait in a ifdef that excludes Solaris and BPF. What do you think?
> 
> [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf ([basic.fundamental])

TBH, feels like a hack to me: if `int8_t` can be either `char` (if signed) or `signed char`, the code should deal with both possibilities.

That said, it would certainly unbreak the Solaris builds which for now is my primary concern.  AFAIK there's no lldb port to Solaris, though
I vaguely recall that there was an attempt in the Illumos (OpenSolaris derivative) community.  On top of that, those static lists are quite
fragile IMO: something like this should be feature-based instead, but that is an all too common problem all over LLVM.

What about your idea of simply casting to `int8_t` in lldb instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79745