Skip to content

Conversation

@chris-eibl
Copy link
Member

@chris-eiblchris-eibl commented Mar 28, 2025

/*
* If Windows has bluetooth support, include bluetooth constants.
*/
#ifdefAF_BTH
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, we're in the #else of #ifndef MS_WINDOWS here and we most probably should indent, but I wanted to keep the diff small.

# include<ws2bth.h>
# ifdef__clang__
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wpragma-pack"
Copy link
MemberAuthor

@chris-eiblchris-eiblMar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix clang-cl warning:

the current #pragma pack alignment value is modified in the included file [-Wpragma-pack] 

This will silence 16 occurrences in https://github.com/python/cpython/actions/runs/14044831663/job/39366560293?pr=131690#step:4:60.

Actually, pshpack1.h takes care of this warning

#if ! (defined(lint) || defined(RC_INVOKED)) #if ( _MSC_VER >= 800&& !defined(_M_I86)) || defined(_PUSHPOP_SUPPORTED) #pragma warning(disable:4103) #if !(defined( MIDL_PASS )) || defined( __midl ) #pragma pack(push,1) #else#pragma pack(1) #endif#else#pragma pack(1) #endif#endif/* ! (defined(lint) || defined(RC_INVOKED)) */

using #pragma warning(disable:4103), but clang-cl only supports disable for some warnings so far (and doesn't support suppress for those few supported, see also #131821 (comment)).

A different approach would be, to entirely get rid of pshpack1.h, since we are in the MS_WINDOWS code path here. We could do a simple #pragma pack(push,1), which clang-cl understands and neither MSVC nor clang-cl would warn about, but

  • I wanted to do the minimal needed stuff to silence clang ...
  • ... without having to care about other compilers (mingw, etc), which might then be broken (see An include file is needed because various compilers do this in different ways in pshpack1.h)

};
# include<poppack.h>
#endif
# ifdef__clang__
Copy link
MemberAuthor

@chris-eiblchris-eiblMar 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Must be done after poppack.h, because there the previous packing is restored and we'd get the same [-Wpragma-pack] warning.

@python-cla-bot
Copy link

All commit authors signed the Contributor License Agreement.

CLA signed

@chris-eibl
Copy link
MemberAuthor

@zooba May I ask you for a review?

@zoobazooba merged commit 59e2330 into python:mainJul 28, 2025
45 checks passed
@chris-eiblchris-eibl deleted the fix_clangcl_socketmodule.h branch August 2, 2025 16:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@chris-eibl@zooba@picnixz