Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes

[Enze Li <enze.li@hotmail.com>]

Hi Manoj,

On Fri, 2022-07-08 at 22:57 -0700, Manoj Gupta wrote:
> I work on ChromeOS and can provide answers to this.
> 
> <vector> does not include provide std::back_inserter as per C++ spec.
> It is provided by <iterator> [1].
> The fact that it worked so far is just an accident that libc++ and
> libstdc++ were including this
> header with <vector>.
> With libc++, it is no longer true. See commit "[libc++] Removes
> unneeded <iterator> includes." in libc++ [2].
> It is therefore not surprising that gdb fails to build. 

Thank you for the detailed explanation.  It seems that we need to
manually compile and install the libc++ from upstream before we can
reproduce this issue.

> 
> 
> On Fri, Jul 8, 2022 at 9:30 PM Enze Li <enze.li@hotmail.com> wrote:
> > Hi Christopher,
> > 
> > Unfortunately, since I don't have a ChromeBook, I can not reproduce
> > the
> > problem you encountered.
> > 
> > I tried to reproduce your problem on a x86_64-linux device and
> > still
> > did not reproduce it with the trunk.  After digging into this
> > deeply, I
> > found that 'gdb/value.c' has already includes the 'vector' header
> > file
> > after preprocessing.
> > 
> > Here's what I got,
> > 
> > value.c
> >   |--defs.h
> >       |--common-defs.h
> >           |--common-utils.h (#include <vector>)

Geez.  I had mistakenly read that we needed to include 'vector' header
file.  Please ignore this part. It must be an oversight. 😂️

Thanks,
Enze

> > 
> > Finally, I noticed that you were compiling gdb-9.2.  I know nothing
> > about the maintenance of closed branch.  You may want to keep PING
> > and
> > maintainers would show up to look at this.
> > 
> 
> Inclusion of <vector> is not guaranteed to provide <iterator>. And I
> believe that the current gdb still does not include <iterator> in
> value.c [3].
> So I am sure that this patch is still needed in trunk.
> 
> Note: Looking at value.c, I think it needs more header includes e.g.
> std::move [4] is used but <utility> is not included.
> 
> [1] https://en.cppreference.com/w/cpp/iterator/back_inserter
> [2] [libc++] Removes unneeded <iterator>
> includes:  https://reviews.llvm.org/rG4cd04d1687f1096990119304a3eb220
> 81ab4bb29
> [3]
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/value.c;h=022fca91a42a8b4d5bccc745b62e642aea8a02ed;hb=refs/heads/master
> [4]: https://en.cppreference.com/w/cpp/utility/move
> 
> Thanks,
> Manoj 
> 
>  
> > Thanks,
> > Enze
> > 
> > On Fri, 2022-07-01 at 14:50 -0700, Christopher Di Bella wrote:
> > > Hi Enze,
> > > 
> > > `std::back_inserter` is defined in <iterator>, which isn't
> > > included
> > > in `gdb/value.c`. Because the C preprocessor imports headers by
> > > copy/pasting the contents at the `#include` site, this means that
> > > another standard library header is exposing it (presumably
> > > `<algorithm>`).
> > > 
> > > We experienced this on ChromeOS, which uses Clang and libc++ to
> > > build
> > > things.
> > > 
> > > ```
> > > /tmp/portage/sys-devel/gdb-9.2.20200923-r7/work/gdb-
> > > 9.2/gdb/value.c:1648:52: error: no member named 'back_inserter'
> > > in
> > > namespace 'std'
> > >        std::move (iter + 1, all_values.end (), std::back_inserter
> > > (result));
> > >                                                ~~~~~^
> > >  1 error generated.
> > > ```
> > > 
> > > On Fri, 1 Jul 2022 at 06:45, Enze Li <enze.li@hotmail.com> wrote:
> > > > On Tue, 2022-06-28 at 01:04 +0000, Christopher Di Bella via
> > > > Gdb-
> > > > patches
> > > > wrote:
> > > > > `std::back_inserter` is defined in `<iterator>`, which is
> > > > > currently
> > > > > being transitively included by one of the other headers. This
> > > > > is
> > > > > causing
> > > > > gdb to fail to build on certain platforms, which is fixed by
> > > > > explicitly
> > > > > including it.
> > > > 
> > > > Hi Christopher,
> > > > 
> > > > I didn't get your point through the commit message.  Can you
> > > > clarify
> > > > exactly on which platforms, and what build error you
> > > > encountered? 
> > > > So
> > > > that I can reproduce the issue.
> > > > 
> > > > Thanks,
> > > > Enze
> > > > 
> > > > 
> > > > > ---
> > > > >  gdb/value.c | 1 +
> > > > >  1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/gdb/value.c b/gdb/value.c
> > > > > index 022fca91a42..ba7ae1a0e18 100644
> > > > > --- a/gdb/value.c
> > > > > +++ b/gdb/value.c
> > > > > @@ -40,6 +40,7 @@
> > > > >  #include "cp-abi.h"
> > > > >  #include "user-regs.h"
> > > > >  #include <algorithm>
> > > > > +#include <iterator>
> > > > >  #include "completer.h"
> > > > >  #include "gdbsupport/selftest.h"
> > > > >  #include "gdbsupport/array-view.h"
> > > > 
> > > 
> > > 
> >