This will break on older macOS versions (essentially any macOS version that predates the existence of iPhone) becauseTARGET_OS_OSX doesn't exist. However, on all versions of iOS,TARGET_OS_OSX willexist with a value of 0. So - my suggested approach here would be:

I can see there's a couple of other places (L99 of this file; L352 of pycore_eval.h) where an analogous change would be required.

I guess the other option would be to modify the code that setsTARGET_OS_OSX from:

#  if !defined(TARGET_OS_OSX) #     define TARGET_OS_OSX 1 #  endif

to

#  if !defined(TARGET_OS_OSX) && !(defined(TARGET_OS_IPHONE) && TARGET_OS_IPHONE)#     define TARGET_OS_OSX 1#  endif

This works becauseTARGET_OS_IPHONE isany Apple mobile device (iOS, tvOS, watchOS, visionOS);TARGET_OS_IOS is specifically an iPhone/iPad. However, futzing withTARGET_OS_OSX seems like asking for trouble, so the explicit approach would be my preferred option.

I think you may be missing the context that this file is importing this header where we are doing the dance:

Yes - I did miss that import would be in effect. Given that imported definition, the form you've got in the PR at present shouldwork - but it still makes me a bit twitchy because it's redefining an Apple-provided constant that has otherwise well defined behavior.

This is essentially what bit me here - the logic didn't make sense because I didn't notice the import where the behavior of the Apple-provided constant was being redefined). We've also had examples in the past where someone adds/removes an import of an otherwise unrelated file, and suddenly things break because that file has redefined basic platform constants (or has removed an import that was making those constants work in a predictable way).

If we're going to go down the path of redefining the constant, it might be worth looking at a top-level replacement forTargetConditionals.h that uses those base definitions and ensures they're consistently defined (so we don't need to do thedefined(TARGET_OS_OSX) && TARGET_OS_OSX dance everywhere) - but that's a much bigger set of changes, and then comes with the overhead of making sure that everyone remembers touse the updated definitions, rather than importingTargetConditionals.h.

So - call my preference for using the definitions-as-provided a "strong opinion, weakly held". With the indentation nitpick flagged inpycore_ceval.h, I can live with this as-is.