On Fri, 31 May 2024 at 16:52, Alexandre Oliva <ol...@adacore.com> wrote:
>
> On May 31, 2024, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> > On 31/05/24 11:07 -0300, Alexandre Oliva wrote:
> >> --- a/libstdc++-v3/include/pstl/pstl_config.h
> [...]
> >> -#if defined(__clang__)
> >> +#if defined(__GLIBCXX__) ? defined(_GLIBCXX_CLANG) : defined(__clang__)
>
> > This file is also imported from upstream, like Ryu and fast_float.
>
> Oh, yeah, I should have mentioned this one in the proposed commit
> message.
>
> The problem here was that it wasn't clear c++config would always be
> included, so I figured I'd better be conservative.
>
> > I don't think having a "spurious" definition of _PSTL_CLANG_VERSION
> > here actually matters.
>
> Yeah, no, it's the other macros guarded by __clang__ that I'm concerned
> about, and since the version macro could replace it, I went for it.
>
> > So either don't change this line at all, or just do a simple
> > s/__clang__/_GLIBCXX_CLANG/
>
> If c++config can be counted on, I'd be happy to do that, but I couldn't
> tell that it could.

You can't reach pstl_config.h without going through one of the
standard headers, which all include c++config.h

>
> > Does the vxworks toolchain need to support the PSTL headers?
>
> Maybe we can do without them.  I really don't know.  Olivier?
>
> > If not, we could just ignore this file, so the local changes don't
> > need to be re-applied when we import a new version of the header from
> > upstream.
>
> That sounds desirable indeed.  This change is supposed to spare us
> (AdaCore) from precisely this sort of trouble, so it wouldn't be fair if
> it made this very kind of trouble for our upstream.
>
> >> --- a/libstdc++-v3/include/std/ranges
>
> >> -#ifdef __clang__ // LLVM-61763 workaround
> >> +#ifdef _GLIBCXX_CLANG // LLVM-61763 workaround
>
> > This one doesn't matter, since making these members public for a "fake
> > clang" doesn't really hurt anything. For consistency maybe it makes
> > sense to use _GLIBCXX_CLANG anyway.
>
> Yeah, uniformity would be good to simplify checking for no new
> appearances of __clang__, and to set the example to avoid accidental
> additions thereof.
>
> >> --- a/libstdc++-v3/include/std/variant
>
> >> -#if defined(__clang__) && __clang_major__ <= 7
> >> +#if defined(_GLIBCXX_CLANG) && __clang_major__ <= 7
>
> > I think we could drop this kluge entirely, clang 7 is old now, we
> > generally only support the most recent 3 or 4 clang versions.
>
> Fine with me, but I'd do that in a separate later patch, so that this
> goes in, and if it gets backported, it will cover this change, rather
> than miss it.  Though, as you say, it doesn't matter much either way.
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Reply via email to