[PATCH] D67656: [llvm-objcopy] Add --set-section-alignment

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/tools/llvm-objcopy/ELF/binary-input.test:120
+# ALIGN-NOT: }
+# ALIGN:     AddressAlignment: 8
----------------
We sometimes use something like:
```
# ALIGN:      Name: .data
# ALIGN:      AddressAlignment:
# ALIGN-SAME: 8
```

Its a bit more straightforward way probably.


================
Comment at: test/tools/llvm-objcopy/ELF/set-section-alignment.test:27
+# RUN:   FileCheck --check-prefix=MULTIPLE %s
+# MULTIPLE: error: --set-section-alignment set multiple times for section '.foo'
+
----------------
The number of spaces between ":" and "error" is different in these 4 tests. I'd suggest stick to a single space everywhere.
(or to align this message if your aim was to have a vertical aligned "error: " lines).

btw, should this one be a error? Tools sometimes allows overriding the options (e.g. linker), it does not seem to be a huge problem to set
the alignment multiple times.


================
Comment at: test/tools/llvm-objcopy/ELF/set-section-alignment.test:39
+  - Name:  .bar
+    Type:  SHT_NOBITS
----------------
What about adding a section with alignment != 0 and a test that shows it can be dropped to 0?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67656