Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make __libcpp_verbose_abort() noexcept like std::terminate() #109151

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dougsonos
Copy link
Contributor

Make __libcpp_verbose_abort() noexcept (it is already noreturn), to match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope. (See #99656)

@dougsonos dougsonos requested a review from a team as a code owner September 18, 2024 14:53
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 18, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 18, 2024

@llvm/pr-subscribers-libcxx

Author: Doug Wyatt (dougsonos)

Changes

Make __libcpp_verbose_abort() noexcept (it is already noreturn), to match std::terminate(). Clang's function effect analysis can use this to ignore such functions as being beyond its scope. (See #99656)


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

1 Files Affected:

  • (modified) libcxx/include/__verbose_abort (+2-1)
diff --git a/libcxx/include/__verbose_abort b/libcxx/include/__verbose_abort
index 244278aec652d2..45a0add493c17d 100644
--- a/libcxx/include/__verbose_abort
+++ b/libcxx/include/__verbose_abort
@@ -21,7 +21,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // This function should never be called directly from the code -- it should only be called through
 // the _LIBCPP_VERBOSE_ABORT macro.
 [[__noreturn__]] _LIBCPP_AVAILABILITY_VERBOSE_ABORT _LIBCPP_OVERRIDABLE_FUNC_VIS
-_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2) void __libcpp_verbose_abort(const char* __format, ...);
+_LIBCPP_ATTRIBUTE_FORMAT(__printf__, 1, 2) void __libcpp_verbose_abort(const char* __format, ...)
+_NOEXCEPT;
 
 // _LIBCPP_VERBOSE_ABORT(format, args...)
 //

Copy link

github-actions bot commented Sep 18, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@dougsonos dougsonos marked this pull request as draft September 18, 2024 14:57
@dougsonos
Copy link
Contributor Author

Oops, I forgot to make the implementation noexcept too. This highlights that the question here is whether changing this interface to noexcept is going to break anything...

@ldionne ldionne marked this pull request as ready for review September 18, 2024 16:02
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, however this is technically a breaking change because in the past we have advertised __libcpp_verbose_abort as being overridable (that's why it was a weak def).

We'd also want to add a release note for this change in libcxx/docs/ReleaseNotes/20.rst, probably under Deprecations and Removals.

Pinging @llvm/libcxx-vendors for awareness.

@dougsonos
Copy link
Contributor Author

Any pointers on what's going on with the test builds? I click Details and just see "job failed".

@ldionne
Copy link
Member

ldionne commented Sep 23, 2024

Any pointers on what's going on with the test builds? I click Details and just see "job failed".

Our CI is currently in a really bad state. Workers are failing to connect. This has been going on for a few days and we're trying to resolve the issue. Sorry for the inconvenience.

@ldionne
Copy link
Member

ldionne commented Sep 25, 2024

Please rebase onto main to re-trigger CI. The CI instability should be resolved now.

match  std::terminate(). Clang's function effect analysis can use this
to ignore such functions as being beyond its scope.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants