THRIFT-6250: Silence the avoidable warnings in the MSVC CI build - #3853
Open
slachiewicz wants to merge 1 commit into
Open
slachiewicz wants to merge 1 commit into
slachiewicz wants to merge 1 commit into
Conversation
Client: build - thriftl.ll: include <stdint.h> in a %top block. Flex defines INT8_MIN and the other limit macros itself when not compiled as C99, guarded by #ifndef, and MSVC's stdint.h then redefined them (9 x C4005 per run). - lib/py/setup.py: pass -std=c++11 only to non-MSVC compilers; cl.exe warned D9002 and ignored it. - CMakeLists.txt: set CMP0144 to NEW next to CMP0074. The MSVC build passes LIBEVENT_ROOT and CMake 3.27+ warned that find_package ignores it. - FindClangTools.cmake: declare FPHSA_NAME_MISMATCHED; the module finds three tools under names that differ from its own by design. - SecurityTest.cpp: cast away the shared_ptr that exists only to make the constructor throw (C4858). Left for later, see the ticket: LNK4098 from libevent built /MT (image change, after THRIFT-6237), C4244 in protocol.tcc (THRIFT-6069 rewrites the file), and CS0114 in the generated netstd Xception. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
slachiewicz
marked this pull request as ready for review
September 14, 2026 09:32
slachiewicz
requested review from
Jens-G,
emmenlau,
fishy and
mhlakhani
as code owners
September 14, 2026 09:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA: THRIFT-6250
Client: build
The MSVC CI job prints about 400 warning lines per run. This takes out the families that have an in-tree fix; the ticket lists what each one is and what is deliberately left for later.
What the diff does not show:
INT8_MINand the other limit macros itself when it is not compiled as C99, which a C++ compile never is, and guards each with#ifndef. MSVC'sstdint.h, included later, then redefined them. The%topblock puts the header first, so the guards skip. winflexbison 2.5.24, which the image installs, is flex 2.6.4 and has the same guards.LIBEVENT_ROOTis passed by build.bat, and CMake 3.27 or later warns three times thatfind_packageignores upper-case<PACKAGENAME>_ROOTvariables until CMP0144 is NEW. The policy guard follows the CMP0074 one above it.FPHSA_NAME_MISMATCHEDis honoured since CMake 3.17 and ignored before, so it stays within the 3.16 minimum.LNK4098lines come from libevent built/MTand needSET CL=/MDinbuild-libevent.bat, which rebuilds the Docker image, so that waits for THRIFT-6237: Shrink the MSVC Docker image #3844.protocol.tcc:995is rewritten by THRIFT-6069: python: add fastbinary decode_binary_from_bytes #3594 and THRIFT-6069: python: use a flat fastbinary encode buffer #3596.AI_V4MAPPEDis THRIFT-6249.Verified:
cmake -Wdevconfigure on macOS with CMake 4.4.3 → no dev warnings;thrift-compilertarget builds and generates the tutorial; the generatedthriftl.cchas#include <stdint.h>on line 2, ahead of the guards.[skip ci]anywhere in the commit message to free up build resources.This change was created with AI assistance.