juce-framework / JUCE

JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, LV2 and AAX audio plug-ins.
https://juce.com
Other
6.3k stars 1.67k forks source link

[Bug]: Review all uses of __clang_major__ #1379

Open ryandesign opened 2 months ago

ryandesign commented 2 months ago

Detailed steps on how to reproduce the bug

Several places in the code check __clang_major__ without apparently being aware that there are two compilers called clang: the one released by llvm.org and the one released by Apple, and they have different version numbering schemes. They have different development teams and different release schedules so there is no exact correspondence between llvm.org clang and Apple clang version numbers but you can see the approximate correspondence on Wikipedia.

One example:

https://github.com/juce-framework/JUCE/blob/ae5144833e852815d61642af87c69b9db44984f7/modules/juce_dsp/widgets/juce_WaveShaper.h#L72

This check was added in eb80465aa910a84d3a834de2b113019fa4213d4e to exclude Apple clang versions prior to 10 to work around a build problem with Apple clang 9.1, so this check will be inaccurate if an llvm.org version of clang is used on macOS.

Another example:

https://github.com/juce-framework/JUCE/blob/ae5144833e852815d61642af87c69b9db44984f7/modules/juce_core/system/juce_CompilerSupport.h#L52-L54

This check comes from b3a4d54a72e4e80aa0ee5cefd1b57ffd195ee643 which was intended to exclude compilers that don't support C++17, so you were clearly talking about llvm.org clang 6; Apple clang 6 predates llvm.org clang 6 by four years and does not support C++17.

__clang_major__ is used in additional places in the code that I have not evaluated.

Every time before you check __clang_major__ you must first establish which of the two editions of clang you're using. If __clang__ is defined, you're using one of the two clangs: If __apple_build_version__ is defined, Apple clang is being used; otherwise llvm.org clang is being used. Once you know which clang is used, you can then compare based on its version numbering scheme.

Finally, making decisions based on compiler versions is not the best idea. If you need to know the C++ version, check the value of __cplusplus. If you need to know if a compiler can do a particular thing, use __has_attribute, __has_builtin, __has_extension, __has_feature, etc. rather than making assumptions based on the compiler version.

What is the expected behaviour?

compiler version checks should be aware of the difference between the two clangs

Operating systems

macOS

What versions of the operating systems?

all

Architectures

x86_64, ARM, Other, 64-bit, 32-bit

Stacktrace

No response

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

No response

Testing on the develop branch

I have not tested against the develop branch

Code of Conduct