◐ Shell
clean mode source ↗

gh-131591: Handle includes for iOS in remote_debugging.c by pablogsal · Pull Request #132050 · python/cpython

Yes - I did miss that import would be in effect. Given that imported definition, the form you've got in the PR at present should work - 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 for TargetConditionals.h that uses those base definitions and ensures they're consistently defined (so we don't need to do the defined(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 to use the updated definitions, rather than importing TargetConditionals.h.

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