Differential D16816

ECM: standardise clang detection
ClosedPublic
Actions

Authored by rjvbb on Nov 11 2018, 9:48 AM.

Details

Reviewers

None

Group Reviewers

Build System

Commits

R240:d42cc5dd7c4e: Use `MATCHES "Clang"` to detect clang

Summary

Scanning the ECM for something that could explain a symbol visibility issue I'm seeing with AppleClang but not with Clang I noticed that this compiler isn't always detected the same way.

This patch ensures that CMAKE_<LANG>_COMPILER_ID MATCHES "Clang" is used throughout, i.e. MATCHES instead of STREQUAL.
This includes 1 or 2 cases where AppleClang is unlikely to be the compiler in use (for instance because of AND NOT APPLE) but I think it's better to use the same detection principle everywhere if not only to prevent inappropriate copy/paste edits in the future.

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

Build Status

Buildable 4820
Build 4838: arc lint + arc unit

rjvbb created this revision.Nov 11 2018, 9:48 AM

Restricted Application edited subscribers, added: kde-buildsystem, kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptNov 11 2018, 9:48 AM

rjvbb requested review of this revision.Nov 11 2018, 9:48 AM

Harbormaster completed remote builds in B4820: Diff 45268.Nov 11 2018, 9:48 AM

The commit message is wrong. It should be Use STREQUAL "Clang" to detect the clang for compatibility with Apple systems.

Otherwise LGTM. +1

Don't worry, the commit message would have looked like that.

Don't worry, the commit message would have looked like that.
Or rather, it will say

Use MATCHES "Clang" to detect clang for compatibility with Apple systems.

because I'm porting away from using STREQUAL (and there's no need for an article before clang ;) )

This revision was not accepted when it landed; it landed in state Needs Review.Nov 12 2018, 10:21 AM

Closed by commit R240:d42cc5dd7c4e: Use `MATCHES "Clang"` to detect clang (authored by rjvbb). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			kde-modules/KDECMakeSettings.cmake (2 lines)
M			kde-modules/KDECompilerSettings.cmake (4 lines)
M			kde-modules/KDEFrameworkCompilerSettings.cmake (2 lines)
M			modules/ECMEnableSanitizers.cmake (2 lines)