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

[Manoj Gupta <manojgupta@google.com>]

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.


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>)
>
> 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/rG4cd04d1687f1096990119304a3eb22081ab4bb29
[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"
> > >
> >
> >
>
>