[libcxx-commits] [PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D112921#3473632 <https://reviews.llvm.org/D112921#3473632>, @pcwang-thead wrote:

> In D112921#3473022 <https://reviews.llvm.org/D112921#3473022>, @ldionne wrote:
>
>> (BTW I strongly support this patch, I just think we should do it properly on all platforms from the start :-)
>
> I couldn't agree with you more, but I have no idea how to implement it. :-(

I was thinking about doing what we do for aligned allocation here: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Darwin.cpp#L2652-L2673

I think this should work:

  bool Darwin::isSizedDeallocationUnavailable() const {
    llvm::Triple::OSType OS;
  
    if (isTargetMacCatalyst())
      return TargetVersion < sizedDeallocMinVersion(llvm::Triple::MacOSX);
    switch (TargetPlatform) {
    case MacOS:
      OS = llvm::Triple::MacOSX;
      break;
    case IPhoneOS:
      OS = llvm::Triple::IOS;
      break;
    case TvOS:
      OS = llvm::Triple::TvOS;
      break;
    case WatchOS:
      OS = llvm::Triple::WatchOS;
      break;
    }
  
    return TargetVersion < sizedDeallocMinVersion(OS);
  }
  
  // This could go in clang/include/clang/Basic/AlignedAllocation.h with a suitable rename, or another header
  inline llvm::VersionTuple sizedDeallocMinVersion(llvm::Triple::OSType OS) {
    switch (OS) {
    default:
      break;
    case llvm::Triple::Darwin:
    case llvm::Triple::MacOSX: // Earliest supporting version is 10.12.
      return llvm::VersionTuple(10U, 12U);
    case llvm::Triple::IOS:
    case llvm::Triple::TvOS: // Earliest supporting version is 10.0.0.
      return llvm::VersionTuple(10U);
    case llvm::Triple::WatchOS: // Earliest supporting version is 3.0.0.
      return llvm::VersionTuple(3U);
    case llvm::Triple::ZOS:
      return llvm::VersionTuple(); // All z/OS versions have no support.
    }
  
    llvm_unreachable("Unexpected OS");
  }

Alternatively, @rjmccall 's approach might be better, but I'm not familiar with what's done for `ObjCRuntime` so I can't comment. I'm personally neutral on what approach is taken.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112921/new/

https://reviews.llvm.org/D112921