Re: [PATCH] Add std::deque shrink_to_fit test

[François Dumont]

AFAICT I've never got proper validation for this small patch.

Is it ok to commit ?

Thanks


On 14/04/2025 22:25, François Dumont wrote:

On 14/04/2025 08:29, Tomasz Kaminski wrote:

On Sun, Apr 13, 2025 at 12:13 PM François Dumont <frs.dum...@gmail.com> wrote:

    On 11/04/2025 08:36, Tomasz Kaminski wrote:

    On Thu, Apr 10, 2025 at 10:47 PM Jonathan Wakely
    <jwak...@redhat.com> wrote:

        On 10/04/25 22:36 +0200, François Dumont wrote:
        >After running the test with -fno-exceptions option we
        rather need this
        >patch.
        >
        >Ok to commit ?
        >
        >François
        >
        >
        >On 10/04/2025 21:08, François Dumont wrote:
        >>Hi
        >>
        >>    No problem detected now that we really test std::deque
        >>shrink_to_fit implementation.
        >>
        >>    libstdc++: Add std::deque<>::shrink_to_fit test
        >>
        >>    The existing test is currently testing std::vector.
        Make it test
        >>std::deque.
        >>
        >>    libstdc++-v3/ChangeLog:
        >>
        >>            *
        >>testsuite/23_containers/deque/capacity/shrink_to_fit.cc:
        Adapt test
        >>            to check std::deque shrink_to_fit method.
        >>
        >>Tested under Linux x64.
        >>
        >>Ok to commit ?
        >>
        >>François

        >diff --git
        a/libstdc++-v3/testsuite/23_containers/deque/capacity/shrink_to_fit.cc
        b/libstdc++-v3/testsuite/23_containers/deque/capacity/shrink_to_fit.cc
        >index 7cb67079214..9c8b3a926e8 100644
        >---
        a/libstdc++-v3/testsuite/23_containers/deque/capacity/shrink_to_fit.cc
        >+++
        b/libstdc++-v3/testsuite/23_containers/deque/capacity/shrink_to_fit.cc
        >@@ -1,4 +1,5 @@
        > // { dg-do run { target c++11 } }
        >+// { dg-add-options no_pch }

        Tests using replacement_memory_operators.h need:

        // { dg-require-effective-target std_allocator_new }
        // { dg-xfail-run-if "AIX operator new" { powerpc-ibm-aix* } }

        See e.g. 23_containers/unordered_set/96088.cc

    Thanks, I new I needed to add something like that but then forgot
    to amend my test.

        >
        > // 2010-01-08  Paolo Carlini  <paolo.carl...@oracle.com>
        >
        >@@ -19,18 +20,39 @@
        > // with this library; see the file COPYING3.  If not see
        > // <http://www.gnu.org/licenses/>.
        >
        >-#include <vector>
        >+#define _GLIBCXX_DEQUE_BUF_SIZE sizeof(int) * 3

        Couldn't the test just create more elements, instead of
        modifying the
        internals? We should test it using the default parameters, no?

    Sounds better indeed, done using std::__deque_buf_size extension.

    In from_range test I have used a class that contains some padding,
    to be able to fill the deque buffer, while still test with few
    elements,

        struct EightInBuf

        {
      EightInBuf(int x) : elems{x}
      { }

     private:
       int elems[512 / (sizeof(int) * 8)];

      friend constexpr bool operator==(EightInBuf const& lhs, int rhs)
      { return lhs.elems[0] == rhs; }
     };         
        

        

    It is a nice alternative even if you are still relying on the 512
    implementation detail hidden by _GLIBCXX_DEQUE_BUF_SIZE macro.

    This is why I preferred to use__deque_buf_size.

When using _GLIBCXX_DEBUG this function will be defined inside std::__cxx1998 namespace, not in the std directly. There is a macro _GLIBCXX_STD_C that can be used to refer to it, but it works only inside std namespace.

Fixed in this new version.

Ok to commit ?

François