[libcxx-commits] [PATCH] D142842: [libc++] Remove _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

So we just went over the patch again. Basically, this undoes the part of D92397 <https://reviews.llvm.org/D92397> that unconditionally defines `~bad_function_call()` in the dylib, but not the part that adds a separate setting for controlling whether `bad_function_call::what()` is defined in the dylib, and hence whether a key function is provided in the dylib.

In the stable ABI, before this patch:

1. User code will always contain a weak definition of the `bad_function_call` vtable since we did not provide a key-function-declaration in the headers. Hence, no user code should contain hard references to `bad_function_call`'s vtable, destructor or else. This was checked against the apps in the App Store, so I think our understanding is confirmed here.
2. The dylib always provides a strong vtable that contains `~bad_function_call()`, but not `bad_function_call::what()`.

In the stable ABI, after this patch:

1. User code will always contain a weak definition of the vtable too.
2. The dylib won't provide a strong vtable for `bad_function_call` anymore, but that shouldn't be used today as noted above.

In the unstable ABI, before this patch, there was a strong vtable in the dylib with `~bad_function_call()` and `bad_function_call::what()`, and users are relying on it. After the patch, it's the same thing except that we don't define `~bad_function_call()` manually, we let the compiler do it.

After all these considerations, I think this is likely safe to do, and we'll indeed remove a few symbols from the dylib, and clean up our `__config` a bit. Please update the ABI list and explain the above in your commit message + in the ABI list change log. I'd like to see this one last time to double check before it goes out.



================
Comment at: libcxx/src/functional.cpp:27
 
 _LIBCPP_END_NAMESPACE_STD
----------------
You will need to grep for `Address Sanitizer doesn't instrument weak symbols on Linux.` and adjust the `UNSUPPORTED` there. It should become something like `UNSUPPORTED: asan && libcpp-abi-version=2`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142842