[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

This revision was automatically updated to reflect the committed changes. Closed by commit rG5122e828701c: [driver][darwin] Dont use -platform_version flag by default (PR44813) (authored by hans). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

hans added a comment. In D74784#1894484 , @dmajor wrote: > Thanks! I'm still working on getting commit access, would one of you be able > to submit for me? I've committed as 5122e828701c88f8d53ee881bc68f3904454d154

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dmajor added a comment. Thanks! I'm still working on getting commit access, would one of you be able to submit for me? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74784/new/ https://reviews.llvm.org/D74784 ___ cfe-commits mailing list

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. LGTM to me too. With @steven_wu and me on board, I don't think you need to wait for @arphaman. Thanks for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74784/new/

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. Thanks. LGTM. I will let @arphaman to approve this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74784/new/ https://reviews.llvm.org/D74784 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dmajor updated this revision to Diff 246771. dmajor edited the summary of this revision. dmajor added a comment. Updated the tests: three cases for {default, old, new} linkers in the platform-version tests; left alone the tests not specifically targeting this path. CHANGES SINCE LAST ACTION

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. In D74784#1891750 , @dmajor wrote: > @steven_wu, ping, could you clarify about the tests please? You want to have testcase to cover all following 3 cases: - Default (version = 0): not using -platform_version - Old version (0

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dmajor added a comment. @steven_wu, ping, could you clarify about the tests please? >> You should definitely add test for this change. The fact that you change all >> `-mlinker-version=400` to `-mlinker-version=0` but not change any CHECK >> lines means the change is definitely not tested :) >

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dmajor added a comment. In D74784#1882974 , @steven_wu wrote: > I forgot if there is reason to use the option by default at all time (I did > ask that in the previous review but Alex might have given more context > offline). I would really like to

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

steven_wu added a comment. I forgot if there is reason to use the option by default at all time (I did ask that in the previous review but Alex might have given more context offline). You should definitely add test for this change. The fact that you change all `-mlinker-version=400` to

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dexonsmith added a comment. Code change looks correct to me. Thanks for the fix! @arphaman, can you confirm the test changes are reasonable? My instinct would have been, instead of changing all of the 400s to 0s, to just adding a single `RUN` line somewhere to confirm we don't do the wrong

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

dmajor created this revision. dmajor added reviewers: arphaman, steven_wu, dexonsmith. dmajor added projects: clang, LLVM. Herald added a subscriber: cfe-commits. (Note, I don't currently have commit access.) The code in llvmorg-10-init-12188-g25ce33a6e4f is a breaking change for users of older