[CMAKE] Improve link flag compiler support detection
ClosedPublic
Actions

Authored by Fabien on Aug 1 2019, 19:40.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGINGcb1cf81755be: [CMAKE] Improve link flag compiler support detection
rABCcb1cf81755be: [CMAKE] Improve link flag compiler support detection

Summary

The add_linker_flag method is able to prevent from introducing link
time errors, but not warnings. This may cause some noise with clang,
which will issue a -Wunused-command-line-argument warning when an
unknown linker flag is set (then ignore it).

This diff promotes the warning to error so the unused link flag will not
be set.

Test Plan

mkdir buildcmake_osx && cd buildcmake_osx
cmake -GNinja .. -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/OSX.cmake
ninja

Check the following warning disappeared with this patch:

clang: warning: argument unused during compilation: '-pie'
[-Wunused-command-line-argument]

Diff Detail

Repository

rABC Bitcoin ABC

Branch

cmake_check_link_improved

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7027
Build 12101: Bitcoin ABC Buildbot (legacy)
Build 12100: arc lint + arc unit

Event Timeline

Fabien created this revision.Aug 1 2019, 19:40

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 1 2019, 19:40

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7027: Diff 10566.Aug 1 2019, 19:46

Fabien added a child revision: D3793: macOS: Prevent Xcode 9.3 build warnings.Aug 1 2019, 20:36

deadalnix requested changes to this revision.Aug 2 2019, 00:00

deadalnix added inline comments.

cmake/modules/AddCompilerFlags.cmake
134	Why not use remove_compiler_flags ?

This revision now requires changes to proceed.Aug 2 2019, 00:00

deadalnix added inline comments.Aug 2 2019, 00:01

cmake/modules/AddCompilerFlags.cmake
118	You can add the C++ flag.

Fabien requested review of this revision.Aug 2 2019, 07:10

Fabien added inline comments.

cmake/modules/AddCompilerFlags.cmake
134	I was hesitant to use `CMAKE_CXX_FLAGS`. The `CMAKE_REQUIRED_FLAGS` are dedicated to this use case, and this would avoid the removal of the flag if added to `CMAKE_CXX_FLAGS` elsewhere.

deadalnix accepted this revision.Aug 5 2019, 05:24

deadalnix added inline comments.

cmake/modules/AddCompilerFlags.cmake
134	Ok. It still feels like this is doing more or less the same, but on another variable. I think thee is some refactoring that could be done here, considering add_c_compiler_flag, add_cxx_compiler_flag and this are doing VERY similar things, just using a different base variable.

This revision is now accepted and ready to land.Aug 5 2019, 05:24

Closed by commit rABCcb1cf81755be: [CMAKE] Improve link flag compiler support detection (authored by Fabien). · Explain WhyAug 5 2019, 07:18

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABCcb1cf81755be: [CMAKE] Improve link flag compiler support detection.

Fabien mentioned this in D3825: [CMAKE] Fix -Wunused-command-line-argument when adding linker flag.Aug 8 2019, 14:39

Fabien mentioned this in rABCc907fdbf9748: [CMAKE] Fix -Wunused-command-line-argument when adding linker flag.Aug 11 2019, 08:05

Fabien added a commit: rSTAGINGcb1cf81755be: [CMAKE] Improve link flag compiler support detection.Sep 19 2019, 01:36

Fabien mentioned this in rSTAGINGc907fdbf9748: [CMAKE] Fix -Wunused-command-line-argument when adding linker flag.Sep 19 2019, 01:44

PiRK mentioned this in D15579: [depends] darwin.mk backports to support newer clang versions.Feb 29 2024, 10:50

PiRK mentioned this in rABC082d9ee1793e: [depends] darwin.mk backports to support newer clang versions.Mar 1 2024, 11:20