[Bug libstdc++/100147] New: libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[Bug libstdc++/100147] New: libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[dcb314 at hotmail dot com]

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

            Bug ID: 100147
           Summary: libstdc++-v3/include/bits/gslice.h:170: missing check
                    for assignment to self ?
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Static analyser cppcheck says:

libstdc++-v3/include/bits/gslice.h:170:11: warning: 'operator=' should check
for assignment to self to avoid problems with dynamic memory.
[operatorEqToSelf]

Source code is

  inline gslice&
  gslice::operator=(const gslice& __g)
  {
    if (__g._M_index)
      __g._M_index->_M_increment_use();
    if (_M_index && _M_index->_M_decrement_use() == 0)
      delete _M_index;
    _M_index = __g._M_index;
    return *this;
  }

It certainly would be standard C++ practice to make the check for assignment
to self. 

Perhaps there is a good reason why it isn't done here.
If so, there may be some value in documenting why.

[Bug libstdc++/100147] libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It's obviously safe for self-assignment.

[Bug libstdc++/100147] libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2024-02-03

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Due to the aliasing rules for valarrays and slices, I think it would be
undefined to assign a gslice to itself. So the overhead of checking for
something that should never happen to avoid a redundant increment/decrement
pair is a poor trade off. Letting it redundantly inc/dec the reference count is
safe and correct for self-assignment, without adding a branch to handle
"impossible" cases.

I suppose we can add a comment to that effect.

[Bug libstdc++/100147] libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:4e5dc6d9686a34d446147b923fe838389758a512

commit r14-8890-g4e5dc6d9686a34d446147b923fe838389758a512
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sun Feb 4 21:39:11 2024 +0000

    libstdc++: Add comment to gslice::operator=(const gslice&) [PR100147]

    There's no need to check for self-assignment here, it would just add
    extra code for an unlikely case. Add a comment saying so.

    libstdc++-v3/ChangeLog:

            PR libstdc++/100147
            * include/bits/gslice.h (operator=): Add comment about lack of
            self-assignment check.

[Bug libstdc++/100147] libstdc++-v3/include/bits/gslice.h:170: missing check for assignment to self ?

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed