-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Build engine TUs with C++20. #174471
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
Build engine TUs with C++20. #174471
Conversation
There was a problem hiding this 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.
2dd52bb to
24883de
Compare
|
|
||
| if (is_clang) { | ||
| extra_warning_flags_cc = [ | ||
| # TODO(XXXX): Third-party dependency (Angle) needs to patched to work |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
gaaclarke
left a comment
There was a problem hiding this 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 |
There was a problem hiding this comment.
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.
|
@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 |
|
Double checked with @jason-simmons for the update and he says LGTM. Landing. |
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.
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.
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.
The changes:
char8_tis a different type. Fixed those instances.gn formatdoesn'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.