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

[libc++] Make a few test helper constructors explicit #118975

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Dec 6, 2024

No description provided.

@ldionne ldionne requested a review from a team as a code owner December 6, 2024 14:07
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 6, 2024
@ldionne
Copy link
Member Author

ldionne commented Dec 6, 2024

CC @winner245

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

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

1 Files Affected:

  • (modified) libcxx/test/std/containers/sequences/vector/common.h (+2-2)
diff --git a/libcxx/test/std/containers/sequences/vector/common.h b/libcxx/test/std/containers/sequences/vector/common.h
index cd57b6cc3e7235..ff8147ef6b838b 100644
--- a/libcxx/test/std/containers/sequences/vector/common.h
+++ b/libcxx/test/std/containers/sequences/vector/common.h
@@ -20,7 +20,7 @@ struct throwing_t {
   int* throw_after_n_ = nullptr;
   throwing_t() { throw 0; }
 
-  throwing_t(int& throw_after_n) : throw_after_n_(&throw_after_n) {
+  explicit throwing_t(int& throw_after_n) : throw_after_n_(&throw_after_n) {
     if (throw_after_n == 0)
       throw 0;
     --throw_after_n;
@@ -95,7 +95,7 @@ struct throwing_iterator {
   int i_;
   T v_;
 
-  throwing_iterator(int i = 0, const T& v = T()) : i_(i), v_(v) {}
+  explicit throwing_iterator(int i = 0, const T& v = T()) : i_(i), v_(v) {}
 
   reference operator*() {
     if (i_ == 1)

Copy link
Contributor

@winner245 winner245 left a comment

Choose a reason for hiding this comment

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

LGTM upon CI green.

@winner245
Copy link
Contributor

Is the lldb test in stage3 (bootstrapping-build) broken? The failed tests seem completely unrelated to this PR (they also failed in my PR):

Failed Tests (15):
lldb-api :: commands/expression/import-std-module/basic/TestImportStdModule.py
lldb-api :: commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
lldb-api :: commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
lldb-api :: commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
lldb-api :: commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
lldb-api :: commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py
lldb-api :: commands/expression/import-std-module/iterator/TestIteratorFromStdModule.py
lldb-api :: commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
lldb-api :: commands/expression/import-std-module/list/TestListFromStdModule.py
lldb-api :: commands/expression/import-std-module/non-module-type-separation/TestNonModuleTypeSeparation.py
lldb-api :: commands/expression/import-std-module/retry-with-std-module/TestRetryWithStdModule.py
lldb-api :: commands/expression/import-std-module/shared_ptr-dbg-info-content/TestSharedPtrDbgInfoContentFromStdModule.py
lldb-api :: commands/expression/import-std-module/shared_ptr/TestSharedPtrFromStdModule.py
lldb-api :: commands/expression/import-std-module/weak_ptr-dbg-info-content/TestDbgInfoContentWeakPtrFromStdModule.py
lldb-api :: commands/expression/import-std-module/weak_ptr/TestWeakPtrFromStdModule.py

@ldionne
Copy link
Member Author

ldionne commented Dec 6, 2024

Looks like this was broken recently in the bootstrapping build. This is probably caused by a clang change. I'll investigate separately.

In the meantime I think we can consider the CI to be green.

@ldionne ldionne merged commit cd74eba into llvm:main Dec 6, 2024
59 of 64 checks passed
@ldionne ldionne deleted the review/explicit-constructors branch December 6, 2024 20:31
broxigarchen pushed a commit to broxigarchen/llvm-project that referenced this pull request Dec 10, 2024
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