* [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes @ 2022-06-28 1:04 Christopher Di Bella 2022-07-01 13:45 ` Enze Li 2022-07-09 9:10 ` Andrew Burgess 0 siblings, 2 replies; 9+ messages in thread From: Christopher Di Bella @ 2022-06-28 1:04 UTC (permalink / raw) To: gdb-patches; +Cc: manojgupta, Christopher Di Bella `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. --- 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" -- 2.37.0.rc0.161.g10f37bed90-goog ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-06-28 1:04 [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes Christopher Di Bella @ 2022-07-01 13:45 ` Enze Li 2022-07-01 21:50 ` Christopher Di Bella 2022-07-09 9:10 ` Andrew Burgess 1 sibling, 1 reply; 9+ messages in thread From: Enze Li @ 2022-07-01 13:45 UTC (permalink / raw) To: Christopher Di Bella, gdb-patches; +Cc: manojgupta 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" ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-01 13:45 ` Enze Li @ 2022-07-01 21:50 ` Christopher Di Bella 2022-07-09 4:30 ` Enze Li 0 siblings, 1 reply; 9+ messages in thread From: Christopher Di Bella @ 2022-07-01 21:50 UTC (permalink / raw) To: Enze Li; +Cc: gdb-patches, manojgupta 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" > > -- Kind regards, Christopher Di Bella (he/him/his) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-01 21:50 ` Christopher Di Bella @ 2022-07-09 4:30 ` Enze Li 2022-07-09 5:57 ` Manoj Gupta 0 siblings, 1 reply; 9+ messages in thread From: Enze Li @ 2022-07-09 4:30 UTC (permalink / raw) To: Christopher Di Bella; +Cc: gdb-patches, manojgupta 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. 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" > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-09 4:30 ` Enze Li @ 2022-07-09 5:57 ` Manoj Gupta 2022-07-09 8:54 ` Enze Li 0 siblings, 1 reply; 9+ messages in thread From: Manoj Gupta @ 2022-07-09 5:57 UTC (permalink / raw) To: Enze Li; +Cc: Christopher Di Bella, gdb-patches 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" > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-09 5:57 ` Manoj Gupta @ 2022-07-09 8:54 ` Enze Li 2022-07-10 18:52 ` Simon Marchi 0 siblings, 1 reply; 9+ messages in thread From: Enze Li @ 2022-07-09 8:54 UTC (permalink / raw) To: Manoj Gupta; +Cc: Christopher Di Bella, gdb-patches 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" > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-09 8:54 ` Enze Li @ 2022-07-10 18:52 ` Simon Marchi 2022-07-16 12:38 ` Enze Li 0 siblings, 1 reply; 9+ messages in thread From: Simon Marchi @ 2022-07-10 18:52 UTC (permalink / raw) To: Enze Li, Manoj Gupta; +Cc: Christopher Di Bella, gdb-patches On 2022-07-09 04:54, Enze Li via Gdb-patches wrote: > 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. This is trivial enough that it's not really necessary to reproduce to get convinced that such a change would be good. The file uses back_inserter -> C++ says that back_inserter is provided by <iterator> -> make the file include <iterator>. This has no real change of breaking anything. Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-07-10 18:52 ` Simon Marchi @ 2022-07-16 12:38 ` Enze Li 0 siblings, 0 replies; 9+ messages in thread From: Enze Li @ 2022-07-16 12:38 UTC (permalink / raw) To: Simon Marchi, Manoj Gupta; +Cc: Christopher Di Bella, gdb-patches On Sun, 2022-07-10 at 14:52 -0400, Simon Marchi wrote: > On 2022-07-09 04:54, Enze Li via Gdb-patches wrote: > > 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. > > This is trivial enough that it's not really necessary to reproduce to > get convinced that such a change would be good. The file uses > back_inserter -> C++ says that back_inserter is provided by > <iterator> > -> make the file include <iterator>. This has no real change of > breaking anything. Indeed, reproduction is not necessary. I was just curious about building gdb with libc++. I hope my interruptions didn't interfere with the progress of reviewing the patch. Best Regards, Enze > > Simon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes 2022-06-28 1:04 [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes Christopher Di Bella 2022-07-01 13:45 ` Enze Li @ 2022-07-09 9:10 ` Andrew Burgess 1 sibling, 0 replies; 9+ messages in thread From: Andrew Burgess @ 2022-07-09 9:10 UTC (permalink / raw) To: Christopher Di Bella; +Cc: gdb-patches, manojgupta * Christopher Di Bella via Gdb-patches <gdb-patches@sourceware.org> [2022-06-28 01:04:27 +0000]: > `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, Thanks for this fix. If you could update the commit message with the additional information that Manoj provided then repost, and we can get this merged. Thanks, Andrew > --- > 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" > -- > 2.37.0.rc0.161.g10f37bed90-goog > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-16 12:38 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-28 1:04 [PATCH] [PATCH] [gdb] adds `<iterator>` to list of includes Christopher Di Bella 2022-07-01 13:45 ` Enze Li 2022-07-01 21:50 ` Christopher Di Bella 2022-07-09 4:30 ` Enze Li 2022-07-09 5:57 ` Manoj Gupta 2022-07-09 8:54 ` Enze Li 2022-07-10 18:52 ` Simon Marchi 2022-07-16 12:38 ` Enze Li 2022-07-09 9:10 ` Andrew Burgess
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).