Page MenuHomePhabricator

[CMAKE] Refactor the AddCompilerFlags facilities
ClosedPublic

Authored by Fabien on Aug 5 2019, 15:13.

Details

Summary

This adds a bit of code factorization by adding a pair of functions to
add and remove flags to target variable.
There is no behavior change.

Depends on D3800 and D3801.

Test Plan
mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Aug 6 2019, 06:24

Looks good overall, but it looks like there are leftovers.

cmake/modules/AddCompilerFlags.cmake
19 ↗(On Diff #10609)

By convention, private construct are prefixed with _ .

80 ↗(On Diff #10609)

This check is redundant as add_compiler_flags_to_var already does it.

84 ↗(On Diff #10609)

You probably want to call add_compiler_flags_to_var unconditionally here.

This revision now requires changes to proceed.Aug 6 2019, 06:24
cmake/modules/AddCompilerFlags.cmake
19 ↗(On Diff #10609)

I don't want to make it private, It will be used to add debug only flags (see D3803)

Avoid duplicating the checks.

This revision is now accepted and ready to land.Aug 6 2019, 16:15