[
https://issues.apache.org/jira/browse/THRIFT-6325?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6325.
--------------------------------
Fix Version/s: 0.26.0
Assignee: Jens Geyer
Resolution: Fixed
> Let the MSVC builds use /MP again
> ---------------------------------
>
> Key: THRIFT-6325
> URL: https://issues.apache.org/jira/browse/THRIFT-6325
> Project: Thrift
> Issue Type: Bug
> Components: Build Process
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.26.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> h3. Problem
> {{build/cmake/DefinePlatformSpecifc.cmake}} means to hand {{/MP}} to MSVC so
> that the sources of a target compile in parallel:
> {code}
> foreach(lang IN ITEMS C CXX)
> if("CMAKE_${lang}_COMPILER_ID" STREQUAL "MSVC")
> # These flags are not supported (or needed) with Clang-Cl on Windows:
> set(CMAKE_${lang}_FLAGS "${CMAKE_${lang}_FLAGS} /MP") # parallel build
> endif()
> {code}
> The left-hand side is quoted but never dereferenced, so the comparison is
> between the literal string {{CMAKE_CXX_COMPILER_ID}} and {{MSVC}}. The
> top-level {{CMakeLists.txt}} asks for {{cmake_minimum_required(VERSION
> 3.16)}}, which puts CMP0054 on NEW, and under NEW a quoted {{if()}} argument
> is never expanded as a variable. The condition is therefore false for every
> compiler, and {{/MP}} has never reached a command line.
> Reproduced with CMake 3.22:
> {noformat}
> set(CMAKE_CXX_COMPILER_ID "MSVC")
> if("CMAKE_CXX_COMPILER_ID" STREQUAL "MSVC") -> false
> if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") -> true
> CMP0054 = NEW
> {noformat}
> The quoting predates the split of the MSVC and Clang-Cl settings; it is not a
> regression from that change.
> h3. Change
> Dereference the variable in the {{/MP}} condition. One character pair, no
> behavioural change other than MSVC compiling a target's sources in parallel
> again.
> h3. Deliberately out of scope
> The two conditions below it carry the same bug, and repairing them changes
> behaviour well beyond build speed:
> * the {{/W3}} and {{/source-charset:utf-8}} and {{/execution-charset:utf-8}}
> branch also calls {{add_definitions("-DUNICODE -D_UNICODE")}}, which switches
> the TCHAR semantics of every Windows build;
> * the {{elseif}} branches for Clang and GNU would newly apply {{-Wall
> -Wextra}} and {{-fmax-errors=1}} / {{-ferror-limit=1}} across the whole Linux
> and macOS CI, most likely surfacing a large number of warnings at once.
> Each deserves its own ticket and its own review. This one is scoped to
> {{/MP}}.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
