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