bump our minimum supported clang version on Linux (maybe Mac) to something newer
Categories
(Firefox Build System :: General, task)
Tracking
(firefox72 fixed)
| Tracking | Status | |
|---|---|---|
| firefox72 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj, NeedInfo)
References
Details
Attachments
(1 file)
My most recent C++17 try run indicated that clang 4.0 doesn't even support -std=gnu++17. Maybe it supports -std=gnu++1z or something like that. Regardless, bumping to a newer clang version to get real C++17 support seems like a reasonable thing to do.
Options:
- (the play-it-safe option) Bump to whatever supports
-std=gnu++17or whatever claims support for the latest C++17 draft. - (more aggressive) Bump to clang 8.0 (which is what we support on Windows).
I think we can afford to be slightly more aggressive on our minimum clang version than we are on our minimum GCC version:
- Our minimum GCC version needs to be somewhat restrained to make Linux distributions's lives easier.
- People compiling with clang are probably using a new-ish version anyway.
- If they're not using a new-ish version, we have bootstrappable clang toolchains for them to use.
- If they, for some reason, can't use our bootstrappable toolchain...well, they can use GCC (see point 1).
glandium, opinions?
|
||
Comment 1•5 years ago
|
||
Looks like clang 5 added gnu++17:
erahm@shetland$ clang-4.0 test.cpp -std=gnu++17
error: invalid value 'gnu++17' in '-std=gnu++17'
erahm@shetland$ clang-5.0 test.cpp -std=gnu++17
erahm@shetland$
Bumping to clang 8 makes sense to me, I think on Windows we're actually going to want to bump to clang 9 as it has fixes for aarch64.
|
Assignee | |
Comment 2•5 years ago
|
||
I think clang 5 is technically our minimum supported version on Mac...
|
||
Comment 3•5 years ago
|
||
Our minimum GCC version needs to be somewhat restrained to make Linux distributions's lives easier.
And so does clang... because here's the catch: we do require clang on GCC builds, for bindgen.
|
|
Assignee | |
Comment 4•5 years ago
|
||
(In reply to Eric Rahm [:erahm] from comment #1)
Looks like clang 5 added gnu++17:
erahm@shetland$ clang-4.0 test.cpp -std=gnu++17 error: invalid value 'gnu++17' in '-std=gnu++17' erahm@shetland$ clang-5.0 test.cpp -std=gnu++17 erahm@shetland$Bumping to clang 8 makes sense to me, I think on Windows we're actually going to want to bump to clang 9 as it has fixes for aarch64.
Windows already requires clang-cl/clang 8.0; I have no issues bumping there because Windows-specific fixes are still coming in.
Can you see what clang-5.0 -dM -E -x c /dev/null -std=gnu++17 -o - |grep __cplus says?
|
|
||
Comment 5•5 years ago
•
|
||
(In reply to Nathan Froyd [:froydnj] from comment #4)
Can you see what
clang-5.0 -dM -E -x c /dev/null -std=gnu++17 -o - |grep __cplussays?
#define __cplusplus 201703L
|
|
Assignee | |
Comment 6•5 years ago
|
||
So clang 4, for whatever reason, fails to compile things with -std=gnu++1z:
clang is failing to find this overload:
https://searchfox.org/mozilla-central/source/dom/crypto/CryptoBuffer.h#38-43
because it's failing to consider the base classes of the object passed in (I think).
That sort of bug is not something I'm really interested in diving into.
There's also the interesting problem that clang 4 built with GCC 7 has...issues with libstdc++ 7.x headers, due to incomplete C++17 support. So we'd have to build clang 4 with GCC 6...which doesn't really help with trying to provide C++17-compatible headers during the build process.
Looking at our Linux compatibility matrix:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Linux_compatibility_matrix
The only interesting distros that have clang < 5 and are still under active support are:
- RHEL 6/7, which apparently don't have clang 4, so aren't really interesting for Firefox (which is surprising to me?)...but clang 5/gcc 7 are at least available for RHEL 7: https://developers.redhat.com/blog/2018/07/07/yum-install-gcc7-clang/
- Debian 8
- Debian 9 (which does have clang 5 available in stretch-backports: https://packages.debian.org/search?suite=stretch-backports&searchon=names&keywords=clang-5)
- Ubuntu 16.04 (which does have clang 5 in xenial updates: https://packages.ubuntu.com/search?suite=xenial-updates§ion=all&arch=any&keywords=clang-5&searchon=names)
- SLES 12, which apparently doesn't have clang in the first place (?) -- though clang 5 builds on SLES 12, according to LLVM's official downloads: http://releases.llvm.org/download.html#5.0.1
So the only sticking point would be Debian 8, which doesn't even have ESR 68, much less whatever our next ESR (~75?) would be.
So I don't think bumping our minimum clang version would have much impact on Linux distributions. I also think developers' lives would be much easier not having to contend with a compiler with a creaky implementation of C++17 features, if any actually exist at all.
Would you be OK with bumping things to clang 5?
|
|
||
Comment 7•5 years ago
|
||
So the only sticking point would be Debian 8, which doesn't even have ESR 68
It doesn't have it yet, because 60 is still supported. But it will have it soon. OTOH, by the time ESR68 is EOLed, Debian 8 will have been EOLed too.
Note, though, that availability in backports doesn't tell much about stable (wrt Debian 9). However, rust updates will require llvm updates, so clang backports are not much of a stretch. But that's only me talking, that would need more feedback from the involved parties.
NI Sylvestre for clang in Debian, Wolfgang for SLES and Martin for RedHat.
|
||
Comment 9•5 years ago
|
||
For openSUSE raising the requirement is not an issue.
For SLES12 I'm guessing that it can be solved but pointed Martin here to validate.
|
|
Assignee | |
Comment 10•5 years ago
|
||
Apparently it would be really nice for C++17 support if we could go all the way to clang 7, as clang 6 still has some C++17 bugs. So the farther we can push clang support, the better.
|
||
Comment 11•5 years ago
|
||
RedHat/Fedora uses gcc everywhere to build Firefox and we recently use gcc8/clang7 from additional developer repos to build Firefox on all RHELs. Also we can easily update any compiler/toolchain packages by Red Hat compiler collections so I'm fine with the change as it does not impact us.
|
||
Comment 12•5 years ago
|
||
SUSE has clang 7 available as a build-dependency on SLE-12 and SLE-15. We might manage to get clang 8 for the next ESR-release, but staying at <= 7 would probably be better for us.
|
||
Comment 13•5 years ago
|
||
About Debian Jessie (aka 8), there are a bunch of newer versions of clang available here: http://apt.llvm.org/jessie/pool/main/l/ Back in April, I stopped maintaining it for Jessie because Jessie backports aren't available anymore https://lists.debian.org/debian-backports-announce/2018/07/msg00000.html and nobody complained.
Porting more recent version of Rust on Jessie might be also a bunch of work.
So, I recommend we don't support Debian 8 anymore (after 68 is EOL).
For Ubuntu, as the clang packages are the same as apt.llvm.org, we could take care of uploading a new version of clang into xenial-updates.
But I don't know about Rust backporting. I guess Rust upstream have some CI on these LTS version.
|
|
Assignee | |
Comment 14•5 years ago
|
||
I pinged Chris over email last month and I pinged him again today. I think it's fair to say that if we don't get a response by the end of the week, we can assume, given Debian's OK with this, that Ubuntu is too?
|
|
Assignee | |
Comment 15•5 years ago
|
||
We need this for "full" C++17 support (everything is supported, but some
C++17 features still have bugs) and this change also brings Linux into
parity with out Mac requirements.
|
||
Comment 16•5 years ago
|
||
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d64119136e15 raise the minimum clang version to 5; r=#build
|
||
Comment 17•5 years ago
|
||
| bugherder | ||
|
||
Updated•5 years ago
|
Description
•