Non-constant instances have been made constant in data_const.pass.cpp

[2LoS]

Non-constant instances have been made constant in data_const.pass.cpp tests, which verify the correct working of the data() member function for constant instances of the std::vector container (libc++).

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: LoS (2LoS)

Changes

Non-constant instances have been made constant in data_const.pass.cpp tests, which verify the correct working of the data() member function for constant instances of the std::vector container (libc++).

---

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

1 Files Affected:

• (modified) libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp (+11-11)

diff --git a/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
index 885caf272afbf2..05afe0160032ac 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
@@ -39,7 +39,7 @@ TEST_CONSTEXPR_CXX20 bool tests()
         assert(is_contiguous_container_asan_correct(v));
     }
     {
-        std::vector<Nasty> v(100);
+        const std::vector<Nasty> v(100);
         assert(v.data() == std::addressof(v.front()));
         assert(is_contiguous_container_asan_correct(v));
     }
@@ -55,24 +55,24 @@ TEST_CONSTEXPR_CXX20 bool tests()
         assert(is_contiguous_container_asan_correct(v));
     }
     {
-        std::vector<Nasty, min_allocator<Nasty>> v(100);
+        const std::vector<Nasty, min_allocator<Nasty>> v(100);
         assert(v.data() == std::addressof(v.front()));
         assert(is_contiguous_container_asan_correct(v));
     }
     {
-      const std::vector<int, safe_allocator<int>> v;
-      assert(v.data() == 0);
-      assert(is_contiguous_container_asan_correct(v));
+        const std::vector<int, safe_allocator<int>> v;
+        assert(v.data() == 0);
+        assert(is_contiguous_container_asan_correct(v));
     }
     {
-      const std::vector<int, safe_allocator<int>> v(100);
-      assert(v.data() == &v.front());
-      assert(is_contiguous_container_asan_correct(v));
+        const std::vector<int, safe_allocator<int>> v(100);
+        assert(v.data() == &v.front());
+        assert(is_contiguous_container_asan_correct(v));
     }
     {
-      std::vector<Nasty, safe_allocator<Nasty>> v(100);
-      assert(v.data() == std::addressof(v.front()));
-      assert(is_contiguous_container_asan_correct(v));
+        const std::vector<Nasty, safe_allocator<Nasty>> v(100);
+        assert(v.data() == std::addressof(v.front()));
+        assert(is_contiguous_container_asan_correct(v));
     }
 #endif
 

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff f37bee1d929a90dd3dbb67a4a9d0a52400a8a78f 17b23ac1ed8e19939c27866020722e841917697e --extensions cpp -- libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp

View the diff from clang-format here.

diff --git a/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp b/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
index 0cf0b22047..71f1dbb77d 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp
@@ -39,9 +39,9 @@ TEST_CONSTEXPR_CXX20 bool tests()
         assert(is_contiguous_container_asan_correct(v));
     }
     {
-        const std::vector<Nasty> v(100);
-        assert(v.data() == std::addressof(v.front()));
-        assert(is_contiguous_container_asan_correct(v));
+      const std::vector<Nasty> v(100);
+      assert(v.data() == std::addressof(v.front()));
+      assert(is_contiguous_container_asan_correct(v));
     }
 #if TEST_STD_VER >= 11
     {
@@ -55,9 +55,9 @@ TEST_CONSTEXPR_CXX20 bool tests()
         assert(is_contiguous_container_asan_correct(v));
     }
     {
-        const std::vector<Nasty, min_allocator<Nasty>> v(100);
-        assert(v.data() == std::addressof(v.front()));
-        assert(is_contiguous_container_asan_correct(v));
+      const std::vector<Nasty, min_allocator<Nasty>> v(100);
+      assert(v.data() == std::addressof(v.front()));
+      assert(is_contiguous_container_asan_correct(v));
     }
     {
       const std::vector<int, safe_allocator<int>> v;

[2LoS]

The automated checks report a fail on code formatting. However, my changes to code formatting have been made to align the second block of code with the first one. [see libcxx/test/std/containers/sequences/vector/vector.data/data_const.pass.cpp, lines 31 - 45]

[winner245]

The CI is using a newer version of clang-format, which basically indents by 2 spaces. The easiest way to fix the format issue is to run clang-format locally before git push.

[2LoS]

The CI is using a newer version of clang-format, which basically indents by 2 spaces. The easiest way to fix the format issue is to run clang-format locally before git push.

This means I have to bring it back to a 2-spaces indentation?

[winner245]

This means I have to bring it back to a 2-spaces indentation?

Yes. But not just the code you changed, everything in the file should be indented by 2 spaces. FYI, here is a detailed specification: https://llvm.org/docs/CodingStandards.html

[2LoS]

Checks for code formatting are still failing. What should I do?

[winner245]

Click "View the diff from clang-format here." under the github-actions bot. It shows which part of your code is not properly formatted. Currently, operator&() is not formatted properly.

As I mentioned earlier, the easiest way to fix all format issues in one go is to run clang-format locally.

[ldionne]

Actually, in this case please let the formatting CI job fail. We'd rather not reformat whole tests when we're just changing a small part of them.

In the future, we'll want to run clang-format on the test suite to make this smoother, but for now just ignore the failing formatting job for the test files (and please undo any unnecessary formatting changes).

[winner245]

In the future, we'll want to run clang-format on the test suite to make this smoother

@ldionne I see. Thank you for the clarification. I really look forward to that.

@2LoS Sorry for misguiding you. I guess you second commit which only contains adding the const should be good.

[2LoS]

@winner245, considered that the failure if the checks for code formatting can be ignored in this case, is the pull request eligible to be merged?

[winner245]

@winner245, considered that the failure if the checks for code formatting can be ignored in this case, is the pull request eligible to be merged?

The PR looks good to me. However, I don’t have merge access. I suggest we wait for the upcoming clang-format run on all libcxx tests, as mentioned by @ldionne. By then, your formatting issue should be resolved automatically with a rebase onto the main branch, allowing a maintainer to merge your PR.

[ldionne]

LGTM, thanks for the patch! And sorry for the confusion about formatting -- we need to do this but the challenge is that the patch to clang-format the whole test suite is going to be horrendous to review :/. Of course that's not something you should have to worry about, so sorry that you had to deal with this.