This is an archive of the discontinued LLVM Phabricator instance.

[CMake][llvm] avoid conflict w/ (and use when available) new builtin check_linker_flag
ClosedPublic

Authored by radford on Apr 20 2021, 3:34 PM.

Details

Summary

Match the API for the new check_linker_flag and use it directly when available, leaving the old code as a fallback.

Diff Detail

Event Timeline

radford created this revision.Apr 20 2021, 3:34 PM
radford requested review of this revision.Apr 20 2021, 3:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 20 2021, 3:34 PM
JDevlieghere requested changes to this revision.Apr 22 2021, 6:30 PM
JDevlieghere added inline comments.
llvm/cmake/modules/HandleLLVMOptions.cmake
892

I don't think this is CMake's check_linker_flag but rather the one from CheckLinkerFlag.cmake. Given that this requires CMake 3.18 and we support older versios, we will continue to need the wrapper until that gets bumped. I think we should probably update the function and rename it to avoid the name collision (and maybe implement it in terms of check_linker_flag(CXX ... when the CMake version is >=3.18.

This revision now requires changes to proceed.Apr 22 2021, 6:30 PM
radford updated this revision to Diff 340956.Apr 27 2021, 12:55 PM

[CMake][llvm] The first argument to check_linker_flag is the language

Match the API for the new check_linker_flag and use it directly when
available, leaving the old code as a fallback.

Herald added a project: Restricted Project. · View Herald TranscriptApr 27 2021, 12:55 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
radford retitled this revision from [CMake][llvm] The first argument to check_linker_flag is the language to [CMake][llvm] avoid conflict w/ (and use when available) new builtin check_linker_flag.Apr 27 2021, 12:56 PM
radford edited the summary of this revision. (Show Details)
JDevlieghere accepted this revision.Apr 27 2021, 4:06 PM

LGTM. Thanks!

This revision is now accepted and ready to land.Apr 27 2021, 4:06 PM