This will allow us to make osx specific changes easier. Because apple silicon macs also run on aarch64, it was easy to confuse it with iOS.
rdar://75302812
Differential D100157
[compiler-rt] add SANITIZER_OSX aralisza on Apr 8 2021, 5:52 PM. Authored by
Details
This will allow us to make osx specific changes easier. Because apple silicon macs also run on aarch64, it was easy to confuse it with iOS. rdar://75302812
Diff Detail
Event TimelineComment Actions I'm not a big fan of the name but the sensible name (SANTIZER_MAC) is already taken unfortunately.
Comment Actions From the TargetConditionals.h header: TARGET_OS_MAC - Generated code will run under Mac OS X variant TARGET_OS_OSX - Generated code will run under OS X devices TARGET_OS_IPHONE - Generated code for firmware, devices, or simulator TARGET_OS_IOS - Generated code will run under iOS TARGET_OS_TV - Generated code will run under Apple TV OS TARGET_OS_WATCH - Generated code will run under Apple Watch OS TARGET_OS_MACCATALYST - Generated code will run under macOS TARGET_OS_SIMULATOR - Generated code will run under a simulator +----------------------------------------------------------+ | TARGET_OS_MAC | | +---+ +------------------------------------+ +---------+ | | | | | TARGET_OS_IPHONE | | | | | | | | +---------------+ +----+ +-------+ | | | | | | | | | IOS | | | | | | | | | | |OSX| | |+-------------+| | TV | | WATCH | | |DRIVERKIT| | | | | | || MACCATALYST || | | | | | | | | | | | | |+-------------+| | | | | | | | | | | | | +---------------+ +----+ +-------+ | | | | | +---+ +------------------------------------+ +---------+ | +----------------------------------------------------------+ So adding SANITIZER_OSX would mirror this even though the name is a bad (outdated) one. LGTM (with Dan's suggestions)! Note: in some places we use the order of #if-#elif-#else to make the #else mean maOS. Having an explicit macro should be better. Comment Actions Just floating this as on option. We could call the new define SANITIZER_MACOS. pro:
cons:
I don't have strong opinions either way, we should just pick one and stick with it. @aralisza @yln Any opinions on this? Comment Actions I would vote for the "extremes", i.e., either
The second option would leave us in a better state, but is probably not worth the churn. Renaming everything at once in upstream llvm doesn't sound too bad, but one has to consider the downstream implications. Comment Actions Personally, I would prefer option 1
because I don't really want to end up on the git blame for all the lines I rename :( |
clang-format: please reformat the code