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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
