Page MenuHomePhabricator

[libc++] Fix build with gcc 4.8
ClosedPublic

Authored by thomasanderson on Jun 13 2019, 1:12 PM.

Details

Summary

gcc 4.8.4 (but not 5.4.0 or 7.3.0) has trouble initializing errc with {}, giving
the error in [1]. This CL switches to explicitly using errc(0), which gcc 4.8
accepts.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=973723

Diff Detail

Repository
rL LLVM

Event Timeline

thomasanderson created this revision.Jun 13 2019, 1:12 PM

We don't support GCC 4.8. We require GCC 5.

If Chromium is needs GCC 4.8, we should talk about this.

thakis added a subscriber: thakis.Jun 13 2019, 2:00 PM

Are libc++'s supported compilers different from LLVM's? 4.8 is still supported for LLVM (but not for long).

Chromium itself doesn't use gcc at all, but some diaspora projects (e.g. v8, for node) support building with gcc, so we have a bot that builds a "hello world" to make sure that the build config bits to disable clang keep working. Maybe that file should just not include a C++ header but only a C header, or maybe we shouldn't use hermetic libc++ on that bot.

In any case, if this is the only required change, there's little harm in accepting it, no? It solves a problem for us (that we could possibly solve in a different way) and the patch arguably makes the code easier to read too :)

EricWF accepted this revision.Jun 13 2019, 2:45 PM

This change is OK.

But the reason behind it is concerning, because libc++ doesn't support these compilers. Things will break again in the future, and they won't be cleaned up.

GCC 4.8 is not enough of a C++11 compiler to tolerate libc++. We've always had to use 4.9 as the baseline GCC.
We now require 5.1, and I'm planning on pushing for the stricter requirement of only supporting the last three major versions of GCC or Clang.

This revision is now accepted and ready to land.Jun 13 2019, 2:45 PM

To be clear, I didn't actually test on 4.9 or 5.1, so it's possibly broken there too. Anyway, landing this for now if only as a band-aid.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2019, 3:24 PM