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++][test] Fixes for hash<Emplaceable> and value discarding #126566

Merged

Conversation

frederick-vs-ja
Copy link
Contributor

Currently std::hash<Emplaceable>::operator() relies implicit conversion from int to size_t, which makes MSVC compelling. This PR switches to use static_cast.

In flat.map/flat.map.access/at_transparent.pass.cpp, there's one value-discarding use of at that wasn't marked TEST_IGNORE_NODISCARD. This PR adds the missing TEST_IGNORE_NODISCARD.

Currently `std::hash<Emplaceable>::operator()` relies implicit
conversion from `int` to `size_t`, which makes MSVC compelling. This PR
switches to use `static_cast`.

In `flat.map/flat.map.access/at_transparent.pass.cpp`, there's one
value-discarding use of `at` that wasn't marked `TEST_IGNORE_NODISCARD`.
This PR adds the missing `TEST_IGNORE_NODISCARD`.
@frederick-vs-ja frederick-vs-ja added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite labels Feb 10, 2025
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner February 10, 2025 18:15
@llvmbot
Copy link
Member

llvmbot commented Feb 10, 2025

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

Currently std::hash&lt;Emplaceable&gt;::operator() relies implicit conversion from int to size_t, which makes MSVC compelling. This PR switches to use static_cast.

In flat.map/flat.map.access/at_transparent.pass.cpp, there's one value-discarding use of at that wasn't marked TEST_IGNORE_NODISCARD. This PR adds the missing TEST_IGNORE_NODISCARD.


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

2 Files Affected:

  • (modified) libcxx/test/std/containers/Emplaceable.h (+1-1)
  • (modified) libcxx/test/std/containers/container.adaptors/flat.map/flat.map.access/at_transparent.pass.cpp (+1-1)
diff --git a/libcxx/test/std/containers/Emplaceable.h b/libcxx/test/std/containers/Emplaceable.h
index 1a4e14505fb21ed..001886eece7d75e 100644
--- a/libcxx/test/std/containers/Emplaceable.h
+++ b/libcxx/test/std/containers/Emplaceable.h
@@ -45,7 +45,7 @@ struct std::hash<Emplaceable> {
   typedef Emplaceable argument_type;
   typedef std::size_t result_type;
 
-  std::size_t operator()(const Emplaceable& x) const { return x.get(); }
+  std::size_t operator()(const Emplaceable& x) const { return static_cast<std::size_t>(x.get()); }
 };
 
 #endif // TEST_STD_VER >= 11
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.access/at_transparent.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.access/at_transparent.pass.cpp
index 13edca915fd005c..456f12e0c0d29ff 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.access/at_transparent.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.access/at_transparent.pass.cpp
@@ -103,7 +103,7 @@ int main(int, char**) {
     TransparentComparator c(transparent_used);
     std::flat_map<int, int, TransparentComparator> m(std::sorted_unique, {{1, 1}, {2, 2}, {3, 3}}, c);
     assert(!transparent_used);
-    m.at(Transparent<int>{3});
+    TEST_IGNORE_NODISCARD m.at(Transparent<int>{3});
     assert(transparent_used);
   }
 

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@frederick-vs-ja frederick-vs-ja merged commit 998f242 into llvm:main Feb 10, 2025
80 checks passed
@frederick-vs-ja frederick-vs-ja deleted the libcxx-test-emplaceable-discard branch February 10, 2025 23:56
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…vm#126566)

Currently `std::hash<Emplaceable>::operator()` relies implicit
conversion from `int` to `size_t`, which makes MSVC compelling. This PR
switches to use `static_cast`.

In `flat.map/flat.map.access/at_transparent.pass.cpp`, there's one
value-discarding use of `at` that wasn't marked `TEST_IGNORE_NODISCARD`.
This PR adds the missing `TEST_IGNORE_NODISCARD`.
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
…vm#126566)

Currently `std::hash<Emplaceable>::operator()` relies implicit
conversion from `int` to `size_t`, which makes MSVC compelling. This PR
switches to use `static_cast`.

In `flat.map/flat.map.access/at_transparent.pass.cpp`, there's one
value-discarding use of `at` that wasn't marked `TEST_IGNORE_NODISCARD`.
This PR adds the missing `TEST_IGNORE_NODISCARD`.
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. test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants