[Bug c++/112477] New: Assignment of value-initialized iterators differs from value-initialization

[Bug c++/112477] New: Assignment of value-initialized iterators differs from value-initialization

[dangelog at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

            Bug ID: 112477
           Summary: Assignment of value-initialized iterators differs from
                    value-initialization
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dangelog at gmail dot com
  Target Milestone: ---

In debug mode there seems to be a difference between a value-initialized
iterator, and an iterator that gets assigned from a value-initialized iterator.

Testcase: https://gcc.godbolt.org/z/hW7d7Pao3

#define _GLIBCXX_DEBUG
#include <map>

int main() {
    using M = std::map<int, int>;
    using I = M::iterator;

    M map{ {1, 1}, {2, 2} };

    I it1 = map.begin();
    it1 = I{};

    I it2{};
    (void)(it1 == it2); 
}

Results in 

  Error: attempt to compare a singular iterator to a 
  singular (value-initialized) iterator.


It's not entirely clear to me why this shouldn't "just work™", although this is
probably threading the needle; assignment *from* a singular iterator isn't
really discussed in https://eel.is/c++draft/iterator.requirements.general#7 nor
in https://eel.is/c++draft/forward.iterators#2 . 

(One may argue that this last sentence was added by N3644, when iterators still
required copiability, so copying a value-constructed iterator should have value
semantics and the result be indistinguishable from value-initializing...)

[Bug libstdc++/112477] Assignment of value-initialized iterators differs from value-initialization

[dangelog at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

Giuseppe D'Angelo <dangelog at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |libstdc++

--- Comment #1 from Giuseppe D'Angelo <dangelog at gmail dot com> ---
Sorry, I've accidentally selected the wrong component.

[Bug libstdc++/112477] Assignment of value-initialized iterators differs from value-initialization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-12-06
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I agree this should work.

[Bug libstdc++/112477] Assignment of value-initialized iterators differs from value-initialization

[dave.martin at giref dot ulaval.ca]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

Dave Martin <dave.martin at giref dot ulaval.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dave.martin at giref dot ulaval.ca

--- Comment #3 from Dave Martin <dave.martin at giref dot ulaval.ca> ---
I have a very similar issue using operator= (https://godbolt.org/z/cj7Gbn1e8)

#define _GLIBCXX_DEBUG
#include <set>

int main()
{ 
  using S = std::set<int>;

  S set {1};
  auto it = set.begin();
  it = S::const_iterator();

  auto it2 = it; 

  return 0;
}

Results in 

  Error: attempt to copy-construct an iterator from a singular iterator.

I am also confused as to why this shouldn't "just work".

[Bug libstdc++/112477] Assignment of value-initialized iterators differs from value-initialization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Because in C++11 any use of a singular iterator, even copying it, was undefined
behaviour. The debug mode checks have not been properly updated to reflect the
relaxed preconditions in more recent standards.

[Bug libstdc++/112477] Assignment of value-initialized iterators differs from value-initialization

[dave.martin at giref dot ulaval.ca]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #5 from Dave Martin <dave.martin at giref dot ulaval.ca> ---
(In reply to Jonathan Wakely from comment #4)
> Because in C++11 any use of a singular iterator, even copying it, was
> undefined behaviour. The debug mode checks have not been properly updated to
> reflect the relaxed preconditions in more recent standards.

I'm a little confused by your response. I recently found this change in our
test suite when we updated our compilers. Our code works fine in gcc 12.3 but
returns the copy-construct error in 13.2 (as shown in the godbolt example),
both using C++17.

Are you saying the debug mode of gcc 13.2 has not been properly updated to
reflect the changes in C++17?

Because, from my understanding, gcc 13.2 added a check that restrains the
preconditions. 

In other words, you seem to be saying that 13.2 was not updated to relax the
preconditions, but the behaviour I am seeing is that 13.2 added a restraint on
the preconditions.

I think the change comes from this commit:
https://github.com/gcc-mirror/gcc/commit/01b1afdc35c13cbff5cd3d37f9319285ab84b157

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Assignment of               |[13/14 Regression]
                   |value-initialized iterators |Assignment of
                   |differs from                |value-initialized iterators
                   |value-initialization        |differs from
                   |                            |value-initialization
      Known to fail|                            |13.2.0
      Known to work|                            |12.3.0
                 CC|                            |fdumont at gcc dot gnu.org
   Target Milestone|---                         |13.3

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
You didn't mention this is a recent regression, I assumed it was a pre-existing
problem. It seems that the error for Guiseppe's original testcase is also a
recent regression, almost certainly the same bug.

That seems to be an unintended consequence of that change. As far as I can see,
it was supposed to only affect detached iterators, but your example doesn't
have any.

François, please take a look.

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[dangelog at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #7 from Giuseppe D'Angelo <dangelog at gmail dot com> ---
Hi,

To be honest I didn't even notice it was a regression, but you're absolutely
right, I can't reproduce my problem with GCC 12, only with GCC 13 (both in
C++17 mode).

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[fdumont at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

François Dumont <fdumont at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |fdumont at gcc dot gnu.org

--- Comment #8 from François Dumont <fdumont at gcc dot gnu.org> ---
Hi
I'm going to have a look but if you wish to contribute do not hesitate.
Thanks for the report.

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I see that this is actually causing lots of failures for PSTL tests when run
with -D_GLIBCXX_DEBUG

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Francois Dumont <fdumont@gcc.gnu.org>:

https://gcc.gnu.org/g:46afbeb81414302829fbf10c107e5466a3cf44d7

commit r14-7151-g46afbeb81414302829fbf10c107e5466a3cf44d7
Author: François Dumont <fdumont@gcc.gnu.org>
Date:   Wed Jan 10 19:06:48 2024 +0100

    libstdc++: [_GLIBCXX_DEBUG] Fix assignment of value-initialized iterator
[PR112477]

    Now that _M_Detach do not reset iterator _M_version value we need to reset
it when
    the iterator is attached to a new sequence, even if this sequencer is null
when
    assigning a value-initialized iterator. In this case _M_version shall be
resetted to 0.

    libstdc++-v3/ChangeLog:

            PR libstdc++/112477
            * src/c++11/debug.cc
            (_Safe_iterator_base::_M_attach): Reset _M_version to 0 if
attaching to null
            sequence.
            (_Safe_iterator_base::_M_attach_single): Likewise.
            (_Safe_local_iterator_base::_M_attach): Likewise.
            (_Safe_local_iterator_base::_M_attach_single): Likewise.
            * testsuite/23_containers/map/debug/112477.cc: New test case.

    Reviewed-by: Jonathan Wakely <jwakely@redhat.com>

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

--- Comment #11 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Francois Dumont
<fdumont@gcc.gnu.org>:

https://gcc.gnu.org/g:ffc5684a4d3d3c457e5d311e7088f3487cf5083e

commit r13-8212-gffc5684a4d3d3c457e5d311e7088f3487cf5083e
Author: François Dumont <fdumont@gcc.gnu.org>
Date:   Wed Jan 10 19:06:48 2024 +0100

    libstdc++: [_GLIBCXX_DEBUG] Fix assignment of value-initialized iterator
[PR112477]

    Now that _M_Detach do not reset iterator _M_version value we need to reset
it when
    the iterator is attached to a new sequence. Even if this sequence is null
like when
    assigning a value-initialized iterator. In this case _M_version shall be
reset to 0.

    libstdc++-v3/ChangeLog:

            PR libstdc++/112477
            * src/c++11/debug.cc
            (_Safe_iterator_base::_M_attach): Reset _M_version to 0 if
attaching to null
            sequence.
            (_Safe_iterator_base::_M_attach_single): Likewise.
            (_Safe_local_iterator_base::_M_attach): Likewise.
            (_Safe_local_iterator_base::_M_attach_single): Likewise.
            * testsuite/23_containers/map/debug/112477.cc: New test case.

    Reviewed-by: Jonathan Wakely <jwakely@redhat.com>

[Bug libstdc++/112477] [13/14 Regression] Assignment of value-initialized iterators differs from value-initialization

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112477

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #9)
> I see that this is actually causing lots of failures for PSTL tests when run
> with -D_GLIBCXX_DEBUG

I'm still seeing some PSTL failures with -D_GLIBCXX_DEBUG mode, but the
remaining ones were already there in GCC 12 and already reported: PR 90276

So I think this is fixed now, thanks.