Page MenuHomePhabricator
Differential D4584

[CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions
ClosedPublic
Actions

Authored by Fabien on Nov 29 2019, 16:15.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rSTAGING3a12c8538e5b: [CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions
rABC3a12c8538e5b: [CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions
Summary

The CMAKE_<LANG>_FLAGS where not propagated to the parent scope.
The configuration specific variables were correctly scoped if a
configuration is defined, which is our default, but invalid if unset.

Test Plan
ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cmake_fix_remove_flag_scope
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8336
Build 14691: Default Diff Build & Tests
Build 14690: arc lint + arc unit

Event Timeline

Fabien created this revision.Nov 29 2019, 16:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 29 2019, 16:15
Herald added a reviewer: deadalnix. · View Herald Transcript
teamcity added a comment.Nov 29 2019, 16:15

Build Bitcoin-ABC / Diffs / Diff Testing started.

deadalnix requested changes to this revision.Nov 29 2019, 16:20

Considering every single assignation is done to the parent_scope, you know this should have been a macro and not a function.

This revision now requires changes to proceed.Nov 29 2019, 16:20
teamcity added a comment.Nov 29 2019, 16:28

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B8336: Diff 14507.Nov 29 2019, 16:28
Fabien requested review of this revision.Nov 30 2019, 14:11

There is one (hidden) assignation, the BUILD_TYPE_FLAGS variable from string(TOUPPER "CMAKE_C_FLAGS_${CMAKE_BUILD_TYPE}" BUILD_TYPE_FLAGS)

deadalnix accepted this revision.Nov 30 2019, 17:51
This revision is now accepted and ready to land.Nov 30 2019, 17:51
Closed by commit rABC3a12c8538e5b: [CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions (authored by Fabien). · Explain WhyDec 2 2019, 07:38
This revision was automatically updated to reflect the committed changes.
Fabien added a commit: rABC3a12c8538e5b: [CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions.
Fabien added a commit: rSTAGING3a12c8538e5b: [CMAKE] Fix scope issue in the remove_<lang>_compiler_flags() functions.Apr 7 2020, 21:56

Revision Contents
Changeset List

PathSize
cmake/
modules/
6 lines

Diff 14507

View Options

cmake/modules/AddCompilerFlags.cmake

Loading...