[PATCH] D77844: [ObjectYAML][MachO] Add support for relocations

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/MachOEmitter.cpp:339
+  const unsigned IsExtern = R.is_extern;
+  const unsigned Type = R.type;
+  if (IsObjLittleEndian)
----------------
alexshap wrote:
> grimar wrote:
> > I think you can inline these 5 `const unsigned` variables.
> so basically here integer promotion rules come into play, e.g. if i am not mistaken bool would be implicitly casted to int (signed) and then left shifted, since we are shifting to the left everything would be okay (even though int is signed https://en.cppreference.com/w/cpp/language/operator_arithmetic#Bitwise_shift_operators), but I somehow wanted to avoid these hidden implicit transformations and make things explicit.  
I do not mind leaving `bool`s alone (I've not noticed them honestly),
but why not to inline other variables at least to make the code shorter?

```
  const unsigned IsPCRel = R.is_pcrel ? 1 : 0;
  const unsigned IsExtern = R.is_extern ? 1 : 0;
  MRE.r_word1 = ((unsigned)R.symbolnum << 0) | (IsPCRel << 24) |
                ((unsigned)R.length << 25) | (IsExtern << 27) |
                ((unsigned)R.type << 28);
```


================
Comment at: llvm/test/ObjectYAML/MachO/relocations_empty.yaml:8
+
+# CHECK:    Sections:
+# CHECK-NEXT:      - sectname:        __text
----------------
I guess having a one section with no relocations was enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77844