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. > 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
