Code added by D106688 has a problem. It passes the option -bxyz to the system linker as -b xyz xyz (duplication of the string 'xyz' is incorrect). This patch fixes that oversight.
Details
- Reviewers
ZarkoCA hubert.reinterpretcast Whitney cebowleratibm jsji - Group Reviewers
Restricted Project - Commits
- rG41e3ac398c3a: [AIX]: Fix option processing for -b
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/Driver/ToolChains/CommonArgs.cpp | ||
---|---|---|
261 | Can we move the OPT_b block above into the if-else chain after the OPT_z case? The else occurring after the comment strikes me as being very unfortunate code formatting. |
LGTM, Thanks.
clang/test/Driver/Xlinker-args.c | ||
---|---|---|
26 | It would be better if we can tested mixing other options like -z . |
clang/test/Driver/Xlinker-args.c | ||
---|---|---|
26 | 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). |
clang/test/Driver/Xlinker-args.c | ||
---|---|---|
26 | 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. |
Can we move the OPT_b block above into the if-else chain after the OPT_z case? The else occurring after the comment strikes me as being very unfortunate code formatting.