[Lldb-commits] [PATCH] D93421: Fix how ValueObject deals with getting unsigned values

[Raphael Isemann via Phabricator via lldb-commits]

teemperor added a comment.

Whether BOOL is `signed char` or `bool` is apparently depending on the architecture (???). See `ClangExpressionSourceCode::GetText`, Clang's `__OBJC_BOOL_IS_BOOL` macro and then there is also some preprocessor shenanigans for non-Clang ObjC (?) in the `objc.h` header from what I can see. Whether C++ is active is never actually checked anywhere (the `bool` type is coming from `stdbool.h` which is unconditionally includes by `objc.h`).

I think this patch just gets the formatter printing 'YES' because the formatter only knows `0` and `1` as valid values, but when we (correctly) sign-extend the 1-bit `BOOL` (=`signed char`) to 255, it just does it's fall-back logic of printing the integer value (also, that fall-back logic is completely ignoring the specified int format and just always prints as decimal. That seems like a bug...)

Anyway, the real solution is that the current behaviour is actually ... *drum roll* correct:

  $ cat bool.m
  #import <Foundation/Foundation.h>
  
  typedef struct {
      BOOL fieldOne : 1;
      BOOL fieldTwo : 1;
      BOOL fieldThree : 1;
      BOOL fieldFour : 1;
      BOOL fieldfive : 1;
  } BoolBitFields;
  
  int main (int argc, const char * argv[]) {
  
    BoolBitFields myField = {0};
    myField.fieldTwo = YES;
    if (myField.fieldTwo == YES)
      printf("YES\n");
    else
      printf("NO\n");
  
    return 0;
  }
  $ ~/test clang bool.m -o bool -framework Foundation -Wpedantic ; and ./bool
  NO

That bitfield is just not 'YES' but '255' which is neither `YES` nor `NO`. Also there is no Clang warning for this, so this is apparently very cool and very legal code. I am tempted to say the real fix here is to tell people to just not use Objective-C...


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

https://reviews.llvm.org/D93421