[libc++] Remove workaround which allows setting _LIBCPP_OVERRIDABLE_FUNC_VIS externally

[philnik777]

-fvisibility-global-new-delete has been added in Clang 18, so there is no more need to specify the visibility of new/delete via libc++-internal macros.

Some people used a custom new/delete, which requires them to have default visibility, but didn't want to leak any symbols otherwise. Since the new/delete visibility can now be controlled by a compiler flag, there is no reason to set it with out macro.

https://reviews.llvm.org/D128007 originally tried to remove the option to override the macro, but was partially reverted because people actually set this specific macro.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

-fvisibility-global-new-delete has been added in Clang 18, so there is no more need to specify the visibility of new/delete via libc++-internal macros.

---

Full diff: https://github.com/llvm/llvm-project/pull/113139.diff

1 Files Affected:

• (modified) libcxx/include/__config (+1-5)

diff --git a/libcxx/include/__config b/libcxx/include/__config
index fcba56f7e3d5b1..4155fd33dabc44 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -408,11 +408,7 @@ typedef __char32_t char32_t;
 #    define _LIBCPP_EXPORTED_FROM_ABI _LIBCPP_VISIBILITY("default")
 #    define _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS _LIBCPP_VISIBILITY("default")
 #    define _LIBCPP_CLASS_TEMPLATE_INSTANTIATION_VIS
-
-// TODO: Make this a proper customization point or remove the option to override it.
-#    ifndef _LIBCPP_OVERRIDABLE_FUNC_VIS
-#      define _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_VISIBILITY("default")
-#    endif
+#    define _LIBCPP_OVERRIDABLE_FUNC_VIS _LIBCPP_VISIBILITY("default")
 
 #    if !defined(_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS)
 // The inline should be removed once PR32114 is resolved

[philnik777]

CC @rnk

[ldionne]

CC @llvm/libcxx-vendors

@philnik777 Can you please add a short explanation of the history of this flag? I think you removed it and that broke a use case, and now it can be removed. What broke when you originally removed it?

[philnik777]

CC @llvm/libcxx-vendors

@philnik777 Can you please add a short explanation of the history of this flag? I think you removed it and that broke a use case, and now it can be removed. What broke when you originally removed it?

Some people used a custom new/delete, which requires them to have default visibility, but didn't want to leak any symbols otherwise. Since the new/delete visibility can now be controlled by a compiler flag, there is no reason to set it with out macro.

[rnk]

I think Louis is asking for more context links, which would be helpful for reviewers and future readers. My best efforts to find the history turned up https://reviews.llvm.org/D129768, but I couldn't find any more useful history on the Chromium side. It looks like Chromium no longer users this macro, but I can't figure out if it ever did or when that changed.

[ldionne]

LGTM, but please make sure to include the explanation for the history of this change in the PR description (i.e. the commit message once landed).