Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Minor fixes for C++20 compatibility #43674

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Minor fixes for C++20 compatibility #43674

merged 2 commits into from
Jul 18, 2023

Conversation

jiahaog
Copy link
Member

@jiahaog jiahaog commented Jul 14, 2023

Fixes the following errors when building with a C++20 toolchain.

I don't really know C++ that well so any pointers would be appreciated.

For the change to the lambda, we can't follow the recommendation to capture *this explicitly because that wouldn't compile under C++17. This PR changes the semantics slightly to capture surface_ by reference, which might be still okay?

For b/289776142

error: implicit capture of 'this' with a capture default of '=' is deprecated [-Werror,-Wdeprecated-this-capture]
   93 |       if (surface_) {
      |           ^
note: add an explicit capture of 'this' to capture '*this' by reference
   91 |     raster_thread_merger_->SetMergeUnmergeCallback([=]() {
      |                                                     ^
      |                                                      , this
error: bitwise operation between different enumeration types ('CGImageAlphaInfo' and '(unnamed enum at third_party/apple_sdks/xcode_14_2/iphonesimulator/System/Library/Frameworks/CoreGraphics.framework/Headers/CGImage.h:53:9)') is deprecated [-Werror,-Wdeprecated-anon-enum-enum-conversion]
  159 |       static_cast<CGBitmapInfo>(kCGImageAlphaPremultipliedLast |
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  160 |                                 kCGBitmapByteOrder32Big),  // CGBitmapInfo bitmapInfo
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the [CLA].
  • All existing and new tests are passing.

@jiahaog jiahaog marked this pull request as ready for review July 14, 2023 11:55
@jiahaog jiahaog requested a review from chinmaygarde July 14, 2023 11:55
@@ -88,7 +88,7 @@ void Rasterizer::Setup(std::unique_ptr<Surface> surface) {
delegate_.GetParentRasterThreadMerger(), platform_id, gpu_id);
}
if (raster_thread_merger_) {
raster_thread_merger_->SetMergeUnmergeCallback([=]() {
raster_thread_merger_->SetMergeUnmergeCallback([&]() {
Copy link
Member

Choose a reason for hiding this comment

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

Change the capture to [this]

@chinmaygarde chinmaygarde removed their request for review July 14, 2023 17:48
@jiahaog jiahaog added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 18, 2023
@auto-submit auto-submit bot merged commit 4d8704f into flutter:main Jul 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 18, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 18, 2023
…130778)

flutter/engine@116eedf...c6e2328

2023-07-18 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from WZt3P7Fm3_GUvAaDv... to ixKM7wyMrqmPDaQ11... (flutter/engine#43756)
2023-07-18 jiahaog@users.noreply.github.com Minor fixes for C++20 compatibility (flutter/engine#43674)
2023-07-18 jiahaog@users.noreply.github.com Revert "Log dlopen errors in opt builds (#41477)" (flutter/engine#43677)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from WZt3P7Fm3_GU to ixKM7wyMrqmP

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@jiahaog jiahaog deleted the cpp20 branch July 19, 2023 05:21
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
Fixes the following errors when building with a C++20 toolchain.

I don't really know C++ that well so any pointers would be appreciated. 

For the change to the lambda, we can't follow the recommendation to capture `*this` explicitly because that wouldn't compile under C++17. This PR changes the semantics slightly to capture `surface_` by reference, which might be still okay?

For b/289776142

```
error: implicit capture of 'this' with a capture default of '=' is deprecated [-Werror,-Wdeprecated-this-capture]
   93 |       if (surface_) {
      |           ^
note: add an explicit capture of 'this' to capture '*this' by reference
   91 |     raster_thread_merger_->SetMergeUnmergeCallback([=]() {
      |                                                     ^
      |                                                      , this
```

```
error: bitwise operation between different enumeration types ('CGImageAlphaInfo' and '(unnamed enum at third_party/apple_sdks/xcode_14_2/iphonesimulator/System/Library/Frameworks/CoreGraphics.framework/Headers/CGImage.h:53:9)') is deprecated [-Werror,-Wdeprecated-anon-enum-enum-conversion]
  159 |       static_cast<CGBitmapInfo>(kCGImageAlphaPremultipliedLast |
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
  160 |                                 kCGBitmapByteOrder32Big),  // CGBitmapInfo bitmapInfo
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~
```
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
…lutter#130778)

flutter/engine@116eedf...c6e2328

2023-07-18 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from WZt3P7Fm3_GUvAaDv... to ixKM7wyMrqmPDaQ11... (flutter/engine#43756)
2023-07-18 jiahaog@users.noreply.github.com Minor fixes for C++20 compatibility (flutter/engine#43674)
2023-07-18 jiahaog@users.noreply.github.com Revert "Log dlopen errors in opt builds (flutter#41477)" (flutter/engine#43677)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from WZt3P7Fm3_GU to ixKM7wyMrqmP

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants