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

[Raphael Isemann via Phabricator via lldb-commits]

teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.

Some comments about that this still prints 255, but otherwise this is looking good.

(For the others: We agreed offline that adding the missing type checking for BOOL is out of scope for this patch. So the 'fail_value = 0` thing and so on is fine for this revision)



================
Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:1038
   }
-  uint8_t value = (real_guy_sp->GetValueAsUnsigned(0) & 0xFF);
+  uint8_t value = (real_guy_sp->GetValueAsSigned(0) & 0xFF);
   switch (value) {
----------------
That still makes the value unsigned and prints `255` instead of `-1`.  Something like: `int64_t value = real_guy_sp->GetValueAsSigned(0);` should do the trick.


================
Comment at: lldb/test/API/functionalities/data-formatter/boolreference/TestFormattersBoolRefPtr.py:79
+
+        self.expect('p myField',
+                   substrs=['(BoolBitFields)', 'fieldOne = NO', 'fieldTwo = 255', 'fieldThree = NO', 'fieldFour = NO', 'fieldfive = 255'])
----------------
You can do this with `expect_expr` which makes this less fragile (the ValueCheck stuff is backported to the last stable branch, so that shouldn't complicate backporting):
```
lang=python
        # converted YES value after passing through the BOOL : 1 bitfield.
        converted_yes = "-1"
        # BOOL is bool instead of signed char on ARM.
        if isArm:
            converted_yes = "1"
        self.expect_expr('myField', result_type="BoolBitFields",
                         result_children=[
                             ValueCheck(name="fieldOne", summary="NO"),
                             ValueCheck(name="fieldTwo", summary= converted_yes,
                             ValueCheck(name="fieldThree", summary="NO"),
                             ValueCheck(name="fieldFour", summary="NO"),
                             ValueCheck(name="fieldfive", summary= converted_yes)
                         ])
```

Also added the check for when BOOL = bool when this is on ARM (which seems to be how Clang decides if BOOL is bool or signed char). I guess a better check would be to check the underlying type for `BOOL` and see what it actually is, but let's fix this properly when we add type checking to the BOOL summary provider.


================
Comment at: lldb/test/API/functionalities/data-formatter/boolreference/main.mm:8
+    BOOL fieldFour : 1;
+    BOOL fieldfive : 1;
+} BoolBitFields;
----------------
Nit: `fieldfive` instead of `fieldFive`.


================
Comment at: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py:27
                                              'field3 =', '4'])
+        self.expect('p myField',
+                   substrs=['(UCBitFields)', 'fieldOne = \'\\0\'', 'fieldTwo = \'\\x01\'', 'fieldThree = \'\\0\'', 'fieldFour = \'\\0\'', 'fieldfive = \'\\x01\''])
----------------
Same as above with `expect_expr`.


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

https://reviews.llvm.org/D93421