[PATCH] D34574: [Sema] Disable c++17 aligned new and delete operators if not implemented in the deployment target's c++ standard library

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Unlike with sized deallocation (which also requires a new stdlib entry point), it was intentional that this be enabled by default in C++17 mode. Implicit calls are only generated to the aligned forms of `operator new` and `operator delete` in cases where the program would otherwise be broken (due to requiring more alignment than the allocator would provide). A link error seems strictly preferable to silent misalignment at runtime. Plus, you have no way to tell whether the user is providing their own aligned allocation / deallocation functions, so disabling this by fiat for certain targets breaks legitimate use cases -- we need to at least respect the command-line `-faligned-allocation` flag and only use this target-based detection as a default / fallback.

What is the motivation for this change?



================
Comment at: include/clang/Basic/TargetInfo.h:780
+  /// should return true.
+  virtual bool disableAlignedAllocation() const {
+    return false;
----------------
This is the wrong name for a `TargetInfo` member. The `TargetInfo` gets to say whether or not the target is known to support aligned allocation, but it's none of the target's business whether that support is ultimately enabled or disabled. Something like `supportsAlignedAllocation` would make more sense.


================
Comment at: lib/Sema/SemaExprCXX.cpp:2583-2584
                            (Kind == OO_Delete || Kind == OO_Array_Delete);
-    bool HasAlignedVariant = getLangOpts().AlignedAllocation;
+    bool HasAlignedVariant = getLangOpts().AlignedAllocation &&
+                             !Context.getTargetInfo().disableAlignedAllocation();
 
----------------
The driver should be making this determination, not Sema. If -cc1 is invoked with aligned allocation enabled, we should not be overriding it here.


https://reviews.llvm.org/D34574