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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

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

> In D79745#2049234 <https://reviews.llvm.org/D79745#2049234>, @JDevlieghere wrote:
>
> > Hey Rainer,
> >
> > I'm sorry I didn't see your comment earlier. Because of your e-mail address your replies skipped my inbox and ended up in a GCC folder... I'll need to fix that!
>
>
> Ah, that explains a lot ;-)
>
> > If the Solaris build bot is public then reverting is indeed fair game. To the correctness of this patch, if it's allowed for `char` and `uint8_t` to be the same type then this patch is wrong but I'd have to double check this stuff. Regardless, it seems like there's at least one platform where that's the case. Let me see if I can simply work around it and get lldb to build by casting to `uint8_t`.
>
> The builders are at Builder clang-solaris11-sparcv9 <http://lab.llvm.org:8014/builders/clang-solaris11-sparcv9> and Builder clang-solaris11-amd64 <http://lab.llvm.org:8014/builders/clang-solaris11-amd64>, so yes, they are public, although currently on
>  the silent buildmaster.


Okay, I was worried I had missed the build failure e-mails too.

> 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])


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79745