On Sat, Nov 23, 2024 at 7:25 AM Thomas Schwinge <tschwi...@baylibre.com> wrote:
>
> Hi Andrew!
>
> On 2024-11-22T13:15:13-0800, Andrew Pinski <quic_apin...@quicinc.com> wrote:
> > Since diagnostic.h is included in over half of the sources, requiring to 
> > `#define INCLUDE_MEMORY`
> > does not make sense. Instead lets unconditionally include memory in 
> > system.h.
> >
> > The majority of this patch is just removing `#define INCLUDE_MEMORY` from 
> > the sources which currently
> > have it.
>
> Thanks!
>
> > --- a/gcc/system.h
> > +++ b/gcc/system.h
> > @@ -222,6 +222,7 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
> >  #ifdef INCLUDE_FUNCTIONAL
> >  # include <functional>
> >  #endif
> > +# include <memory>
> >  # include <cstring>
> >  # include <initializer_list>
> >  # include <new>
> > @@ -758,13 +759,6 @@ private:
> >  #define LIKELY(x) (__builtin_expect ((x), 1))
> >  #define UNLIKELY(x) (__builtin_expect ((x), 0))
> >
> > -/* Some of the headers included by <memory> can use "abort" within a
> > -   namespace, e.g. "_VSTD::abort();", which fails after we use the
> > -   preprocessor to redefine "abort" as "fancy_abort" below.  */
> > -
> > -#ifdef INCLUDE_MEMORY
> > -# include <memory>
> > -#endif
> >
> >  #ifdef INCLUDE_MUTEX
> >  # include <mutex>
>
> Should we additionally '#pragma GCC poison INCLUDE_MEMORY' (further down
> in the file), so that it's documented, and other uses (external GCC
> parts, WIP work, etc.) also get cleaned up, and nobody remains confused
> about any lingering '#define INCLUDE_MEMORY's (even if they're harmless)?

poison won't work since it is not retroactive.
We could add:
#ifdef INCLUDE_MEMORY
#error "Don't define INCLUDE_MEMORY any more as it is automatic"
#endif

But I suspect that would break plugins which need to compile with say
GCC 14 and GCC 15 and you don't know the version you are compiling for
until after you include plugin.h.
So I think leaving it without the check is the best.

Thanks,
Andrew

>
>
> Grüße
>  Thomas

Reply via email to