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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
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'
+
----------------
grimar wrote:
> 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.
> or to align this message if your aim was to have a vertical aligned "error: " lines

Yes, they are aligned by `error: `.

> btw, should this one be a error?

I agree this does not have to be an error. I'll let the last option win.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D67656