vsk added inline comments.
================ Comment at: compiler-rt/lib/ubsan/ubsan_value.cpp:29 +const char *__ubsan::getObjCClassName(ValueHandle Pointer) { +#if defined(__APPLE__) + // We need to query the ObjC runtime for some information, but do not want ---------------- delcypher wrote: > vsk wrote: > > delcypher wrote: > > > The compiler-rt codebase tends to use `SANITIZER_MAC` macro (defined to > > > be 1 if Apple otherwise it's 0) rather than `__APPLE__`. > > I see. That seems problematic, as it makes it tricky to add macOS-specific > > (or iOS-specific) functionality down the road. I don't think SANITIZER_MAC > > should be defined unless TARGET_OS_MACOS is true. > Sorry I should clarify. `SANITIZER_MAC` is poorly named but it is defined to > be `1` for Apple platforms and `0`. I'm just pointing out the convention that > exists today. You're absolutely right that we might want to do different > things for different Apple platforms but I don't think we want to start doing > a mass re-name until arm64e and arm64_32 support are completed landed in > llvm's master. I think 'SANITIZER_MAC' is confusing, and my preference would be to not use it. `__APPLE__` seems clearer to me, and (IIUC) the plan is to replace usage of 'SANITIZER_MAC' with it down the line anyway? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71491/new/ https://reviews.llvm.org/D71491 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits