Skip to content

Conversation

@chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented Aug 26, 2025

The changes:

  • Creating a bit-mask with different enum values is now a warning. Fixed offending instances.
  • char8_t is a different type. Fixed those instances.
  • Disabling modules in Objective-C requires bypassing the driver.
  • Aggregate initialization fails when there are other constructors for structs. Added explicit constructors.
  • gn format doesn't seem to be run on erstwhile buildroot GN files. Fixes the formatting. But can back this out since it is technically unrelated to C++20.

@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Aug 26, 2025
@chinmaygarde chinmaygarde marked this pull request as draft August 26, 2025 21:11
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the engine build configuration to use C++20 instead of C++17. It also includes several formatting changes and adds compiler flags to handle the transition, such as disabling C++20 modules and suppressing warnings related to deprecated this captures. My review focuses on improving code conciseness and ensuring adherence to the GN style guide for formatting.

@github-actions github-actions bot added a: desktop Running on desktop e: impeller Impeller rendering backend issues and features requests platform-macos labels Aug 27, 2025
@chinmaygarde chinmaygarde marked this pull request as ready for review August 27, 2025 18:19
@chinmaygarde chinmaygarde changed the title Attempt building engine TUs with C++20. Build engine TUs with C++20. Aug 27, 2025

if (is_clang) {
extra_warning_flags_cc = [
# TODO(XXXX): Third-party dependency (Angle) needs to patched to work
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will replace with bug once filed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flutter is currently using a version of ANGLE that is 2 years old.

A few months ago I started working on a roll of ANGLE but never landed it. I'd be interested in reviving that work and rolling forward to a version that does not have this issue.

Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀


if (is_clang) {
extra_warning_flags_cc = [
# TODO(XXXX): Third-party dependency (Angle) needs to patched to work
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flutter is currently using a version of ANGLE that is 2 years old.

A few months ago I started working on a roll of ANGLE but never landed it. I'd be interested in reviving that work and rolling forward to a version that does not have this issue.

@github-actions github-actions bot added the platform-linux Building on or for Linux specifically label Aug 27, 2025
@github-actions github-actions bot added the platform-windows Building on or for Windows specifically label Aug 28, 2025
@chinmaygarde
Copy link
Member Author

@chinmaygarde @gaaclarke I think this is good to go now. Reviews on the additional patches?

There were a couple of windows specific fixups necessary around the use of wchar_t and consexpr use in the generated Flatbuffers headers. I updated the Flatbuffer dependency to pull in fixes made to that repo recently. We hadn't updated since 2022.

@chinmaygarde chinmaygarde requested a review from a team as a code owner August 29, 2025 20:00
@github-actions github-actions bot added platform-ios iOS applications specifically team-ios Owned by iOS platform team labels Aug 29, 2025
@chinmaygarde
Copy link
Member Author

Double checked with @jason-simmons for the update and he says LGTM. Landing.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 2, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Sep 2, 2025
Merged via the queue into flutter:master with commit c6f2de7 Sep 2, 2025
187 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 2, 2025
@chinmaygarde chinmaygarde deleted the attemptc20 branch September 2, 2025 19:07
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 5, 2025
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
The changes:

* Creating a bit-mask with different enum values is now a warning. Fixed
offending instances.
* `char8_t` is a different type. Fixed those instances.
* Disabling modules in Objective-C requires bypassing the driver.
* Aggregate initialization fails when there are other constructors for
structs. Added explicit constructors.
* `gn format` doesn't seem to be run on erstwhile buildroot GN files.
Fixes the formatting. But can back this out since it is technically
unrelated to C++20.
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
The changes:

* Creating a bit-mask with different enum values is now a warning. Fixed
offending instances.
* `char8_t` is a different type. Fixed those instances.
* Disabling modules in Objective-C requires bypassing the driver.
* Aggregate initialization fails when there are other constructors for
structs. Added explicit constructors.
* `gn format` doesn't seem to be run on erstwhile buildroot GN files.
Fixes the formatting. But can back this out since it is technically
unrelated to C++20.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
The changes:

* Creating a bit-mask with different enum values is now a warning. Fixed
offending instances.
* `char8_t` is a different type. Fixed those instances.
* Disabling modules in Objective-C requires bypassing the driver.
* Aggregate initialization fails when there are other constructors for
structs. Added explicit constructors.
* `gn format` doesn't seem to be run on erstwhile buildroot GN files.
Fixes the formatting. But can back this out since it is technically
unrelated to C++20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. platform-ios iOS applications specifically platform-linux Building on or for Linux specifically platform-macos platform-windows Building on or for Windows specifically team-ios Owned by iOS platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants