1394825 - Require libclang 4.0+ for stylo.

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

That would make easier to land bug 1393230, and remove other stylo hacks.

clang 4.0 is already stable (and has been for a while) and it is pretty much everywhere.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Hi Ralph, do you know how easy / hard / doable would this be?

Comment 2

[Gregory Szorc [:gps]]

There is talk of enabling Stylo by default in Nightly in the next few days. If Stylo becomes the Firefox default, then requiring Clang 4.0 for Stylo effectively means bumping Firefox's minimum Clang version from 3.9 to 4.0. I suppose we could still support Clang 3.9 with no Stylo. But we'd have to change the "base toolchains" builds to disable Stylo. And once we drop no Stylo as a supported build configuration, that means we've bumped the minimum Clang dependency to 4.0.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Gregory Szorc [:gps] from comment #2)
> There is talk of enabling Stylo by default in Nightly in the next few days.
> If Stylo becomes the Firefox default, then requiring Clang 4.0 for Stylo
> effectively means bumping Firefox's minimum Clang version from 3.9 to 4.0. I
> suppose we could still support Clang 3.9 with no Stylo. But we'd have to
> change the "base toolchains" builds to disable Stylo. And once we drop no
> Stylo as a supported build configuration, that means we've bumped the
> minimum Clang dependency to 4.0.

Sure, I just supposed it'd be worth to have a bug on file. Please feel free to punt on this until you think it's suitable.

I think it'd be pretty nice for Stylo because that means we can remove a bunch of hacks, but of course I don't know whether that outweights bumping the minimum clang dependency, that's why I asked :)

Comment 4

[Ralph Giles (:rillian)]

I don't think it's particularly hard, assuming there are no regressions. Nathan did the necessary work to our build_clang script, I think in bug 1377214.

We already effectively require libclang 4.0.1 for bindgen on macOS because earlier versions crash there. We just allow compiling the C++ side with an earlier version.

Comment 5

[Chris Peterson [:cpeterson]]

It seems kinda late in the Nightly 57 cycle to bump compiler version if we don't need a particular clang bug fix.

Comment 6

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 7

[Chris Peterson [:cpeterson]]

(In reply to Gregory Szorc [:gps] from comment #2)
> There is talk of enabling Stylo by default in Nightly in the next few days.
> If Stylo becomes the Firefox default, then requiring Clang 4.0 for Stylo
> effectively means bumping Firefox's minimum Clang version from 3.9 to 4.0. I
> suppose we could still support Clang 3.9 with no Stylo. But we'd have to
> change the "base toolchains" builds to disable Stylo. And once we drop no
> Stylo as a supported build configuration, that means we've bumped the
> minimum Clang dependency to 4.0.

We will likely remove Gecko's old style system in Nightly 59 (stylo-everywhere bug 1395112). We can then update from libclang 3.9.0 to something newer like 4.0.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Mike, we still build the base toolchains job with clang 3.9 only for bindgen (since we use GCC for the C++ code IIUC). I don't see anything else using linux64-clang-3.9, is that right?

If so, can we update it to, let's say, clang 5.0? (4.0 would be ok too I guess). I'm happy to figure out the way to tweak the json file in build/build-clang/clang-3.9-linux64.json if you cannot / don't want to do it.

Comment 9

[Mike Hommey [:glandium]]

I actually want to setup builds with the minimum clang we claim to support which is 3.9 at the moment (independently of bindgen). It may or may not be worthwhile to update the minimum clang version we claim to support, but we should at least check out which one(s) work.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Huh, as it turns out, this is blocking some changes I want to do to the style system. We have already so many hacks to workaround libclang 3.9 bugs that are fixed in 4.0 that I took a stab at this.

With 3.9:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1122db9ff531e4c626d9048e27d88e8cc8bc511

With 4.0:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f530666dc2308502b9d3257ce1220d1ff34751e4 (not done yet, but has finished linking libxul, so it did build).

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Attachment: Update minimum clang version to 4.0.

Note that we only use this for libclang at the moment, since
base-toolchains is built with gcc.

libclang 3.9 has a bug that makes bindgen unable to distinguish some typedefs
from the underlying type, which matters for bug 1523071.

We have had quite a few workarounds for this bug and I don't really want to add
more, since in this case it is non-trivial. I think requiring libclang 4.0+ is
reasonable at this point.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6a025216838
Update minimum clang version to 4.0. r=glandium

Comment 13

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/d6a025216838