Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chromium: use clang >= 16 #716

Open
MaxIhlenfeldt opened this issue May 30, 2023 · 24 comments
Open

chromium: use clang >= 16 #716

MaxIhlenfeldt opened this issue May 30, 2023 · 24 comments
Assignees

Comments

@MaxIhlenfeldt
Copy link
Collaborator

MaxIhlenfeldt commented May 30, 2023

Chromium is going to switch to using C++20 soon (one or two months). According to Google's Peter Kasting, "Anything before clang 16 is basically not going to work".

Here's the current clang versions that we use:

  • dunfell: 12
  • kirkstone: 14
  • mickledore: 15

i.e. every Yocto release is currently using a version that's going to cause a lot of problems very soon.

Maybe it's possible to provide clang 16 in a similar way to the recently introduced possibility of using Node 14 on dunfell?

@MaxIhlenfeldt MaxIhlenfeldt changed the title chromium: use clang 14 on dunfell chromium: use clang >= 16 Jun 5, 2023
@MaxIhlenfeldt
Copy link
Collaborator Author

I learned about this by coincidence talking to Peter. It's quite urgent, as the switch to C++20 seems to be only a few major versions away from being released 😅

@kraj
Copy link
Collaborator

kraj commented Jun 6, 2023

using clang16 just for chromium might be fine for dunfell and kirkstone however, this would need work since we wont be able to build rest of OE packages with clang16 without major surgery.

@MaxIhlenfeldt
Copy link
Collaborator Author

we wont be able to build rest of OE packages with clang16 without major surgery.

I would have expected using a newer clang version to be backwards compatible, what kind of problems can arise from updating clang?

using clang16 just for chromium might be fine

Is it possible to use clang16 just for chromium and use the default clang version otherwise?

@kraj
Copy link
Collaborator

kraj commented Jun 15, 2023

we wont be able to build rest of OE packages with clang16 without major surgery.

I would have expected using a newer clang version to be backwards compatible, what kind of problems can arise from updating clang?

Think of this, we are trying to use a compiler released in 2023 to compile packages which were predominantly released
in 2020, clang-16 was not around back then, so these packages may have code which new clang compiler will find errors
in or expose errors which older compiler was incapable of. This is commonly seen issue when mixing newer compiler with older source code of packages.

using clang16 just for chromium might be fine

Is it possible to use clang16 just for chromium and use the default clang version otherwise?

I think yes. If we create dunfell_clang16 then this branch may only be used to compile chromium alone.
this will be troublesome for folks who are using clang as default system compiler with dunfell and also using
chromium and want to upgrade to latest chromium. They will have to switch system compiler to gcc and select
clang only for chromium for best results.

@MaxIhlenfeldt
Copy link
Collaborator Author

This is commonly seen issue when mixing newer compiler with older source code of packages.

I had feared that my expectation regarding backwards compatibility might be a bit optimistic, thanks for confirming.

this will be troublesome for folks who are using clang as default system compiler with dunfell and also using
chromium and want to upgrade to latest chromium. They will have to switch system compiler to gcc and select
clang only for chromium for best results.

Why is that? Is it not possible to use clang 12 as the default system compiler and clang 16 for Chromium only? Either way, I suppose it'd be a reasonable restriction, given that the alternative is not supporting Chromium on dunfell at all.

@MaxIhlenfeldt
Copy link
Collaborator Author

Updating to m115 gives more errors that presumably wouldn't happen with clang 16:

| In file included from ../../chrome/test/chromedriver/capabilities.cc:5:
| In file included from ../../chrome/test/chromedriver/capabilities.h:10:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/map:541:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__node_handle:63:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/memory:846:
| In file included from /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/allocate_at_least.h:13:
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/allocator_traits.h:298:9: error: no matching function for call to 'construct_at'
|         _VSTD::construct_at(__p, _VSTD::forward<_Args>(__args)...);
|         ^~~~~~~~~~~~~~~~~~~
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__config:658:17: note: expanded from macro '_VSTD'
| #  define _VSTD std
|                 ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/vector:818:21: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<BrandVersion>>::construct<BrandVersion, const std::string &, const std::string &, void, void>' requested here
|     __alloc_traits::construct(this->__alloc(), _VSTD::__to_address(__tx.__pos_),
|                     ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/vector:1631:9: note: in instantiation of function template specialization 'std::vector<BrandVersion>::__construct_one_at_end<const std::string &, const std::string &>' requested here
|         __construct_one_at_end(_VSTD::forward<_Args>(__args)...);
|         ^
| ../../chrome/test/chromedriver/capabilities.cc:358:16: note: in instantiation of function template specialization 'std::vector<BrandVersion>::emplace_back<const std::string &, const std::string &>' requested here
|         brands.emplace_back(*brand, *version);
|                ^
| /home/gitlab-runner/yocto-chromium/builds/ci-chromium-wayland-qemuarm64-mickledore/tmp-glibc/work/cortexa57-oe-linux/chromium-ozone-wayland/115.0.5790.102-r0/recipe-sysroot/usr/include/c++/v1/__memory/construct_at.h:33:38: note: candidate template ignored: substitution failure [with _Tp = BrandVersion, _Args = <const std::string &, const std::string &>]: no matching constructor for initialization of 'BrandVersion'
| _LIBCPP_HIDE_FROM_ABI constexpr _Tp* construct_at(_Tp* __location, _Args&&... __args) {
|                                      ^
| 1 error generated.

(I suspect the substitution error mentioned at the end doesn't happen with clang >= 16.)

As already mentioned, there will be more errors as slowly the code base uses more and more C++20 features (this might not even be the only error of this kind in m115). What's the plan for moving to using clang 16 on dunfell, kirkstone and mickledore?

@rakuco
Copy link
Collaborator

rakuco commented Jul 28, 2023

FWIW, I've been toying with a dunfell-clang14 branch that's basically cherry-picking a ton of commits from meta-clang's kirkstone branch. chromium-x11 and chromium-ozone-wayland built fine, but I haven't tried running the binaries yet.

@MaxIhlenfeldt
Copy link
Collaborator Author

That sounds promising! But I fear using clang 14 won't get rid of all the errors, as even mickledore with clang 15 is causing problems :/

@rakuco
Copy link
Collaborator

rakuco commented Jul 28, 2023

That's probably true. At this point I'm trying to do this as a learning experience; if this works, we can have both dunfell and kirkstone using the same compiler version, in which case moving both to a newer version.

With that said, clang 15 is less than a year old, so my only fear is that we'll always spend a non-trivial amount of time chasing compiler failures simply because Chromium's always using LLVM's main branch. Building the Chromium recipe with Chromium's LLVM is probably a no-go :/

@MaxIhlenfeldt
Copy link
Collaborator Author

if this works, we can have both dunfell and kirkstone using the same compiler version, in which case moving both to a newer version.

Ah, good point. That does sound useful.

so my only fear is that we'll always spend a non-trivial amount of time chasing compiler failures simply because Chromium's always using LLVM's main branch

It did work relatively well in the past, didn't it? I think what's currently causing the increased amount of problems is upstream's switch to C++20, which isn't well supported before clang 16.

@MaxIhlenfeldt
Copy link
Collaborator Author

How should we proceed here now that meta-clang has a dunfell-clang14 branch? The update to 117 once more needs many build fixes that wouldn't be needed with clang 16. And while these fixes usually are trivial, they still take up a lot of my time 🙁

@rakuco
Copy link
Collaborator

rakuco commented Sep 27, 2023

I think the course of action would be figuring out which meta-clang branch has clang 16 and trying to backport any required changes to a kirkstone-clang16 branch and then try to replicate the same thing to a dunfell-clang16 branch. Easier said than done though 😄

I don't have much spare time to work on this myself, but can try to help review any changes.

@MaxIhlenfeldt
Copy link
Collaborator Author

Sounds reasonable!

@kraj would you have time for that work? If not, I suppose I could give it a try.

@kraj
Copy link
Collaborator

kraj commented Sep 27, 2023

Sounds reasonable!

@kraj would you have time for that work? If not, I suppose I could give it a try.

we use clang16 on mickledore and moved master to clang17, dunfell will be EOL in Apr 24 so lets see if we can live with clang14

@MaxIhlenfeldt
Copy link
Collaborator Author

we use clang16 on mickledore

https://github.com/kraj/meta-clang/blob/mickledore/recipes-devtools/clang/clang.inc#L7 shows clang 15 being used, is this still WIP?

lets see if we can live with clang14

kirkstone is also still on clang 14 and will be supported until April 2026, so we definitely need to update to clang 16. And iiuc, meta-clang's dunfell-clang14 and kirkstone branches are quite similar, so if we get clang 16 to work on kirkstone, we might also get it to work on dunfell? There will be another seven Chromium releases until April 2024, I think that's enough to warrant at least trying.

@kraj
Copy link
Collaborator

kraj commented Sep 28, 2023

I meant to say clang15 not 16.

@MaxIhlenfeldt
Copy link
Collaborator Author

Thanks for the clarification!

Do you have time to look into updating mickledore and kirkstone to clang 16?

@kraj
Copy link
Collaborator

kraj commented Sep 28, 2023

Thanks for the clarification!

Do you have time to look into updating mickledore and kirkstone to clang 16?

Not in coming 2-4 weeks. Perhaps after October

@MaxIhlenfeldt
Copy link
Collaborator Author

@kraj do you think you'll have time for this soon?

@MaxIhlenfeldt
Copy link
Collaborator Author

It was just announced that many C++20 features are now allowed in Chromium, so maintaining support for clang 14 will probably get very hard very soon.

@MaxIhlenfeldt
Copy link
Collaborator Author

MaxIhlenfeldt commented Feb 7, 2024

It seems like with Chromium 121 it's not really feasible to make it work with clang < 16. I will focus on getting nanbield and master to compile, but the amount of patches needed for kirkstone seems too high. (e.g. just look at https://crrev.com/c/5027747 and its relation chain)

@kraj if possible, can you please take a look at providing a meta-clang/kirkstone-clang16 branch?

@kraj
Copy link
Collaborator

kraj commented Feb 7, 2024

@MaxIhlenfeldt I guess we have to bite the bullet, perhaps its better to move to clang-18 in one go. So perhaps we can live with that for a while.

@MaxIhlenfeldt
Copy link
Collaborator Author

@MaxIhlenfeldt I guess we have to bite the bullet, perhaps its better to move to clang-18 in one go. So perhaps we can live with that for a while.

You're right, probably longer than with clang 16. If clang 18 is possible, that would be great of course!

@rwmacleod
Copy link
Contributor

@MaxIhlenfeldt @kraj Is there something that either @rjanani-p or I can do to help this along?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants