[PATCH] D107786: [AIX]: Fix option processing for -b

Jinsong Ji via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 9 17:17:48 PDT 2021


jsji added inline comments.


================
Comment at: clang/test/Driver/Xlinker-args.c:26
 // LINUX: "--no-demangle" "-e" "_start" "one" "two" "three" "four" "-z" "five" "-r" {{.*}} "-T" "a.lds"
-// AIX: "-b" "one" 
+// AIX: "-b" "one" "-b" "two"
 // NOT-AIX: error: unsupported option '-b' for target 'powerpc-unknown-linux'
----------------
etiotto wrote:
> jsji wrote:
> > It would be better if we can tested mixing other options like `-z` .
> I think the test is adequate as it guarantees options passed via "-b" are not going to be duplicated (which is the problem this PR addresses). 
Yes, it is testing what it tries to fix. However, it does NOT test what it *might* break. This is somehow similar to last patch, it does test what it wants -- passing down the option, but it did not test whether it pass more than needed.

The fix changes code related to handling of `-z`, so there is some risk of breaking `-z`, adding tests will ensure that the fix is good enough, and also prevent future regressions.

But yeah, as I said it is *better* to do that, not a must, depending on the quality level.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107786



More information about the cfe-commits mailing list