GH-45325: [C++] Improved error message when using clang++ without clang #45326
Conversation
|
|
WillAyd
force-pushed
the
fix-clang++-compile
branch
from
cec542b to
bd6aa35
Compare
WillAyd
changed the title
There was a problem hiding this comment.
Thanks, the fix is correct but given that this is an issue that was completely resolved in ccache 3.4 in 2018 (ccache/ccache#188) we should be good just removing that entire thing!
github-actions
bot
added
awaiting committer review
|
(my comment was about the earlier version of the fix ^^) |
|
Yea thanks. We can also remove that and that helps with the configure stage, but I noticed that the program still fails to compile when you mix compilers |
WillAyd
force-pushed
the
fix-clang++-compile
branch
2 times, most recently
from
dd6538b to
51e2039
Compare
| if(CMAKE_C_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_C_COMPILER_ID STREQUAL "Clang") | ||
| set(UsingClang TRUE) | ||
| else() | ||
| set(UsingClang FALSE) | ||
| endif() | ||
|
|
||
| # Check for Clang C++ compiler | ||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") | ||
| set(UsingClangPlusPlus TRUE) | ||
| else() | ||
| set(UsingClangPlusPlus FALSE) | ||
| endif() | ||
|
|
||
| if(${UsingClangPlusPlus} AND NOT ${UsingClang}) | ||
| message(FATAL_ERROR "When using clang++ you must also use clang") | ||
| endif() |
There was a problem hiding this comment.
Can we use if(NOT CMAKE_C_COMPILER_ID STREQUAL CMAKE_CXX_COMPILER_ID)?
There was a problem hiding this comment.
Ah yeah that's nice:
CC=gcc CXX=clang++ cmake .
-- The C compiler identification is GNU 13.3.0
-- The CXX compiler identification is Clang 18.1.3
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
CMake Error at CMakeLists.txt:6 (message):
Use same compiler for C and C++
WillAyd
force-pushed
the
fix-clang++-compile
branch
from
51e2039 to
fd0e85a
Compare
github-actions
bot
added
awaiting change review
| @@ -23,6 +23,12 @@ include(CheckCXXSourceCompiles) | |||
|
|
|||
| message(STATUS "System processor: ${CMAKE_SYSTEM_PROCESSOR}") | |||
|
|
|||
| if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang") | |||
There was a problem hiding this comment.
Do we need this check?
In general, we can't mix C/C++ compilers from different vendores, right?
(Can we mix g++ and clang?)
There was a problem hiding this comment.
Yea mixing g++ and clang is the problem, or vice versa. If you just set -DCMAKE_CXX_COMPILER=clang++ then clang++ will be used for cpp files and gcc will be used for c (assuming gcc is the default)
There was a problem hiding this comment.
but wouldn't the following catch that?
if(NOT CMAKE_CXX_COMPILER_ID STREQUAL CMAKE_C_COMPILER_ID)
Why only perform the check if the CMAKE_CXX_COMPILER_ID is clang?
There was a problem hiding this comment.
Ah I see what you are saying. I don't know CMake well enough to know if those labels are always expected to be the same. Happy to make that change.
This originally started by locating the "bug" in SetupCxxFlags.cmake, hence the condition in the first place
github-actions
bot
added
awaiting changes
Rationale for this change
This resolves a rather cryptic error message encountered when trying to compile with clang++
What changes are included in this PR?
Changing the CMake configuration that adds flags to only add clang-specific command line arguments when clang is being used alongside clang++
Are these changes tested?
Locally yes
Are there any user-facing changes?
No - this is for developers