On Mon, Jan 27, 2025 at 07:31:35PM +0000, Sérgio Basto via devel wrote: > I just want check, if I'm thinking correctly before submitting a fix in > gtest package > > The problem is on Rawhide I have this warning that make other packages > fail to build [1] > > gtest source [2] source get __cplusplus value and include <version> or > #include <ciso646> depending on __cplusplus value . > > On rawhide I got the warning on Fedora 41 don't but __cplusplus of GCC > compiler is the same (201703) > > My proposal is to change the comparison from 202002L to 201703L > -#if GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L && \ > +#if GTEST_INTERNAL_CPLUSPLUS_LANG >= 201703L && \ That is not correct. <version> is actually not guaranteed to be there for C++17, it was only added in https://wg21.link/p0754r2 in 2018 (so e.g. for GCC since GCC 9.1). __has_include is supported by GCC since 2014 (so GCC 5.1). So your change would break building with GCC 5.1 to 8.5. I think if it did e.g. #if (GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L && \ !defined(__has_include)) || \ (GTEST_INTERNAL_CPLUSPLUS_LANG >= 201703L && \ GTEST_INTERNAL_HAS_INCLUDE(<version>)) #include <version> #endif it would be better, C++20 should guarantee there is <version> (but also that __has_include is there, but this stuff attempts to cover also the cases of the incremental development of the standard features). I don't really see the point of including <ciso646>, it is a useless header which doesn't contain anything since its introduction and has been removed without deprecation in C++20. I think the rationale some people give is that it is the smallest C++ header (contains nothing) which still includes some basic header with some macros (in the libstdc++ case it is <bits/c++config.h>, in libcxx case it is <__config>). But e.g. in the libstdc++ case, that header doesn't really define any feature test macros, the __cpp_* macros are predefined by the compiler and __cpp_lib_* are defined by the individual headers which provide that functionality or (when it exists) in <version>. And this header then has #include <ctype.h> // for isspace, etc #include <stddef.h> // for ptrdiff_t #include <stdio.h> #include <stdlib.h> #include <string.h> #include <cerrno> // #include <condition_variable> // Guarded by GTEST_IS_THREADSAFE below #include <cstdint> #include <iostream> #include <limits> #include <locale> #include <memory> #include <ostream> #include <string> // #include <mutex> // Guarded by GTEST_IS_THREADSAFE below #include <tuple> #include <type_traits> #include <vector> so I really don't see the point, including cerrno will include everything what ciso646 provided, cstdint as well, etc. Or go with // Detect C++ feature test macros as gracefully as possible. // MSVC >= 19.15, Clang >= 3.4.1, and GCC >= 4.1.2 support feature test macros. #if GTEST_INTERNAL_HAS_INCLUDE(<version>) || \ (GTEST_INTERNAL_CPLUSPLUS_LANG >= 202002L && !defined(__has_include)) #include <version> // C++20 and later #else #include <cerrno> // Pre-C++20 #endif and leave the #include <cerrno> later on. Jakub -- _______________________________________________ devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
