Skip to content

Conversation

@thurstond
Copy link
Contributor

Fixes hwasan buildbot failure
(https://lab.llvm.org/buildbot/#/builders/55/builds/7536/steps/10/logs/stdio) introduced in #125433 by excluding this test for hwasan, similar to the existing exclusion of asan.

Fixes hwasan buildbot failure
(https://lab.llvm.org/buildbot/#/builders/55/builds/7536/steps/10/logs/stdio) introduced in llvm#125433 by excluding this test for hwasan, similar to the existing exclusion of asan.
@thurstond thurstond added the skip-precommit-approval PR for CI feedback, not intended for review label Feb 25, 2025
@thurstond thurstond requested a review from a team as a code owner February 25, 2025 20:14
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Feb 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 25, 2025

@llvm/pr-subscribers-libcxx

Author: Thurston Dang (thurstond)

Changes

Fixes hwasan buildbot failure
(https://lab.llvm.org/buildbot/#/builders/55/builds/7536/steps/10/logs/stdio) introduced in #125433 by excluding this test for hwasan, similar to the existing exclusion of asan.


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

1 Files Affected:

  • (modified) libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp (+1)
diff --git a/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp b/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp
index 9ab8296d49af1..0d0c63058d656 100644
--- a/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp
+++ b/libcxx/test/std/thread/futures/futures.async/thread_create_failure.pass.cpp
@@ -10,6 +10,7 @@
 
 // ASan seems to try to create threadsm which obviouly doesn't work in this test.
 // UNSUPPORTED: asan
+// UNSUPPORTED: hwasan
 
 // UNSUPPORTED: c++03
 

@thurstond thurstond merged commit 5e4938a into llvm:main Feb 25, 2025
14 of 26 checks passed
@ldionne
Copy link
Member

ldionne commented Feb 26, 2025

@thurstond As a matter of process, please wait for the approval of @llvm/reviewers-libcxx before merging patches to libc++. Also, the intent of the skip-precommit-approval label appears to be for e.g. Draft PRs where you only want to get CI feedback, not to exclude a patch from having review requirements. In this case the patch is extremely trivial, so you could have pinged someone on Discord to get a quick approval.

@thurstond
Copy link
Contributor Author

thurstond commented Feb 26, 2025

@ldionne My apologies for not being familiar with the libc++ process. I had been following the general LLVM patch reversion policy:

"This policy covers all llvm.org subprojects, including Clang, LLDB, libc++, etc.
...
You are allowed to commit patches without approval which you think are obvious. This is clearly a subjective decision — we simply expect you to use good judgement. Examples include: fixing build breakage, reverting obviously broken patches, documentation/comment changes, any other minor changes."
(https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy + https://llvm.org/docs/DeveloperPolicy.html#introduction)

This patch qualified IMO, both as fixing a build breakage and as a minor change.

If libc++ process varies from this general policy, could you please make a note in the docs accordingly? I suspect I won't be the last person to be unaware of libcxx's process and accidentally step on their toes.

@fmayer
Copy link
Contributor

fmayer commented Feb 26, 2025

Also, the intent of the skip-precommit-approval label appears to be for e.g. Draft PRs where you only want to get CI feedback, not to exclude a patch from having review requirements.

I don't think this is accurate. This is e.g. for '[NFC]' that you want to upload as a PR. The name is skip precommit approval.

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. skip-precommit-approval PR for CI feedback, not intended for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants