[PATCH] D122976: Bump minimum toolchain version

[Tobias Hieta via Phabricator via llvm-commits]

thieta added inline comments.


================
Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:100-101
     endif()
     # Test for libstdc++ version of at least 5.1 by checking for std::iostream_category().
     # Note: We should check _GLIBCXX_RELEASE when possible (i.e., for GCC 7.1 and up).
     check_cxx_source_compiles("
----------------
jyknight wrote:
> jyknight wrote:
> > As this change increments the minimum GCC version to 7, we should switch to using `_GLIBCXX_RELEASE` to check the libstdc++ version. This is better than using `__GLIBCXX__` dates, because the dates are updated for every patch release on every branch, so it cannot actually distinguish between branches properly.
> > 
> > The hack of checking iostream_category can go away, as well (it was just a workaround for the unreliability of GLIBCXX).
> > 
> > So, the whole check below should be able to be simplified to:
> > ```
> > #if defined(__GLIBCXX__)
> > #if !defined(_GLIBCXX_RELEASE) || _GLIBCXX_RELEASE < ${LIBSTDCXX_SOFT_ERROR}
> > #error Unsupported libstdc++ version
> > #endif
> > #endif
> > int main() { return 0; }
> > ```
> > 
> Delete the comment; it's now incorrect, and no longer necessary since the test is now obvious.
> Delete unnecessary `#include <iosfwd>`.
> 
iosfwd was needed in order for _GLIBCXX_RELEASE to be defined - maybe we could include a smaller header to get that define - but I couldn't delete it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122976/new/

https://reviews.llvm.org/D122976