Skip to content

Rename ExecutionModelToString to ExecutionModelToStringName.#183461

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
chingjun:rename-function-reflector
Mar 11, 2026
Merged

Rename ExecutionModelToString to ExecutionModelToStringName.#183461
auto-submit[bot] merged 1 commit into
flutter:masterfrom
chingjun:rename-function-reflector

Conversation

@chingjun

Copy link
Copy Markdown
Contributor

The function of the same name ExecutionModelToString was added in spirv.hpp1 in a commit later than the one we pinned to, and it is a build failure when we compile against the later version.

Renaming it to prevent that.

The function of the same name `ExecutionModelToString` was added in
spirv.hpp[1] in a commit later than the one we pinned to, and it is a
build failure when we compile against the later version.

Renaming it to prevent that.
@chingjun chingjun requested a review from jason-simmons March 10, 2026 17:56
@flutter-dashboard

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Mar 10, 2026
@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026

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

Copy link
Copy Markdown
Contributor

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 renames the ExecutionModelToString function to ExecutionModelToStringName to avoid a naming conflict with a dependency. The change is straightforward and addresses the build failure correctly. I have one suggestion to align the code with modern C++ best practices as per the project's style guide.

namespace compiler {

static std::string ExecutionModelToString(spv::ExecutionModel model) {
static std::string ExecutionModelToStringName(spv::ExecutionModel model) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

According to the Google C++ Style Guide, which is followed by this repository, it is preferable to use an anonymous namespace for file-local functions instead of the static keyword. This is the modern C++ approach to limit a function's linkage to its translation unit.

Consider wrapping this function in an anonymous namespace:

namespace {
std::string ExecutionModelToStringName(spv::ExecutionModel model) {
  // ... function body ...
}
} // namespace
References
  1. The repository style guide (line 43) specifies that C++ code should follow the Google C++ Style Guide. The Google C++ Style Guide recommends using anonymous namespaces instead of static for file-local functions. (link)

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026
@auto-submit

auto-submit Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/183461, because - The status or check suite Windows windows_arm_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026
@auto-submit

auto-submit Bot commented Mar 10, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/183461, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Mar 11, 2026
Merged via the queue into flutter:master with commit 6a72c1a Mar 11, 2026
189 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Mar 12, 2026
flutter/flutter@3f400d7...9e36adb

2026-03-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from QD887D4OanteB7UKM... to jJbpv4J_tjW-wuKDq... (flutter/flutter#183584)
2026-03-12 matt.kosarek@canonical.com Adding the SatelliteWindowController interface to `_window.dart` (flutter/flutter#182903)
2026-03-12 engine-flutter-autoroll@skia.org Roll Skia from 0cab3e4ee34b to 46f41493ebf4 (17 revisions) (flutter/flutter#183553)
2026-03-12 engine-flutter-autoroll@skia.org Roll Dart SDK from 8531f7c2bdae to 59be21f25f2d (4 revisions) (flutter/flutter#183547)
2026-03-12 koji.wakamiya@gmail.com [ios][engine] Fix keyboard flicker when switching text fields (flutter/flutter#182661)
2026-03-11 34871572+gmackall@users.noreply.github.com Rename hcpp opt in flag to `enable-hcpp` (flutter/flutter#183151)
2026-03-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#183541)
2026-03-11 97480502+b-luk@users.noreply.github.com Re-enable gpu_test.dart for opengles, but disable specific tests in the file (flutter/flutter#183531)
2026-03-11 jacksongardner@google.com Use CLA-approved email and username for github workflow commits. (flutter/flutter#183540)
2026-03-11 okorohelijah@google.com [integration_test]UIScene Migration (flutter/flutter#182631)
2026-03-11 git@reb0.org ci: Run orchestrator for windows_arm_host_engine on Linux  (flutter/flutter#181075)
2026-03-11 mdebbar@google.com [web] Prevent Firefox auto-updates (flutter/flutter#183330)
2026-03-11 koji.wakamiya@gmail.com Add scrollCacheExtent to PageView (flutter/flutter#180411)
2026-03-11 danny@tuppeny.com Remove redundant VS Code code/tests (flutter/flutter#183506)
2026-03-11 danny@tuppeny.com Roll pub packages (flutter/flutter#183527)
2026-03-11 34871572+gmackall@users.noreply.github.com Reapply "Make HCPP upgrading work for vd/tlhc (#181024)" (#183310) (flutter/flutter#183360)
2026-03-11 victorsanniay@gmail.com Add awaits to dev/ callsites (flutter/flutter#183479)
2026-03-11 stuartmorgan@google.com Standardize review timeline guidance (flutter/flutter#183465)
2026-03-11 chingjun@google.com Rename ExecutionModelToString to ExecutionModelToStringName. (flutter/flutter#183461)
2026-03-11 34871572+gmackall@users.noreply.github.com Fix input mirroring in platform views when system language is RTL, and support is set in manifest. (flutter/flutter#183472)
2026-03-11 engine-flutter-autoroll@skia.org Roll Packages from ee460d6 to ecace66 (10 revisions) (flutter/flutter#183517)

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

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

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
okorohelijah pushed a commit to okorohelijah/packages that referenced this pull request Mar 26, 2026
…r#11233)

flutter/flutter@3f400d7...9e36adb

2026-03-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from QD887D4OanteB7UKM... to jJbpv4J_tjW-wuKDq... (flutter/flutter#183584)
2026-03-12 matt.kosarek@canonical.com Adding the SatelliteWindowController interface to `_window.dart` (flutter/flutter#182903)
2026-03-12 engine-flutter-autoroll@skia.org Roll Skia from 0cab3e4ee34b to 46f41493ebf4 (17 revisions) (flutter/flutter#183553)
2026-03-12 engine-flutter-autoroll@skia.org Roll Dart SDK from 8531f7c2bdae to 59be21f25f2d (4 revisions) (flutter/flutter#183547)
2026-03-12 koji.wakamiya@gmail.com [ios][engine] Fix keyboard flicker when switching text fields (flutter/flutter#182661)
2026-03-11 34871572+gmackall@users.noreply.github.com Rename hcpp opt in flag to `enable-hcpp` (flutter/flutter#183151)
2026-03-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#183541)
2026-03-11 97480502+b-luk@users.noreply.github.com Re-enable gpu_test.dart for opengles, but disable specific tests in the file (flutter/flutter#183531)
2026-03-11 jacksongardner@google.com Use CLA-approved email and username for github workflow commits. (flutter/flutter#183540)
2026-03-11 okorohelijah@google.com [integration_test]UIScene Migration (flutter/flutter#182631)
2026-03-11 git@reb0.org ci: Run orchestrator for windows_arm_host_engine on Linux  (flutter/flutter#181075)
2026-03-11 mdebbar@google.com [web] Prevent Firefox auto-updates (flutter/flutter#183330)
2026-03-11 koji.wakamiya@gmail.com Add scrollCacheExtent to PageView (flutter/flutter#180411)
2026-03-11 danny@tuppeny.com Remove redundant VS Code code/tests (flutter/flutter#183506)
2026-03-11 danny@tuppeny.com Roll pub packages (flutter/flutter#183527)
2026-03-11 34871572+gmackall@users.noreply.github.com Reapply "Make HCPP upgrading work for vd/tlhc (#181024)" (#183310) (flutter/flutter#183360)
2026-03-11 victorsanniay@gmail.com Add awaits to dev/ callsites (flutter/flutter#183479)
2026-03-11 stuartmorgan@google.com Standardize review timeline guidance (flutter/flutter#183465)
2026-03-11 chingjun@google.com Rename ExecutionModelToString to ExecutionModelToStringName. (flutter/flutter#183461)
2026-03-11 34871572+gmackall@users.noreply.github.com Fix input mirroring in platform views when system language is RTL, and support is set in manifest. (flutter/flutter#183472)
2026-03-11 engine-flutter-autoroll@skia.org Roll Packages from ee460d6 to ecace66 (10 revisions) (flutter/flutter#183517)

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

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

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
mboetger pushed a commit to mboetger/flutter that referenced this pull request Mar 26, 2026
…#183461)

The function of the same name `ExecutionModelToString` was added in
spirv.hpp[1] in a commit later than the one we pinned to, and it is a
build failure when we compile against the later version.

Renaming it to prevent that.

[1]:
https://chromium.googlesource.com/external/github.com/KhronosGroup/SPIRV-Cross/+/a4598963b576cb4824b162ccac2c060db395cac4/spirv.hpp#3045
ahmedsameha1 pushed a commit to ahmedsameha1/flutter that referenced this pull request Apr 14, 2026
…#183461)

The function of the same name `ExecutionModelToString` was added in
spirv.hpp[1] in a commit later than the one we pinned to, and it is a
build failure when we compile against the later version.

Renaming it to prevent that.

[1]:
https://chromium.googlesource.com/external/github.com/KhronosGroup/SPIRV-Cross/+/a4598963b576cb4824b162ccac2c060db395cac4/spirv.hpp#3045
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…r#11233)

flutter/flutter@3f400d7...9e36adb

2026-03-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from QD887D4OanteB7UKM... to jJbpv4J_tjW-wuKDq... (flutter/flutter#183584)
2026-03-12 matt.kosarek@canonical.com Adding the SatelliteWindowController interface to `_window.dart` (flutter/flutter#182903)
2026-03-12 engine-flutter-autoroll@skia.org Roll Skia from 0cab3e4ee34b to 46f41493ebf4 (17 revisions) (flutter/flutter#183553)
2026-03-12 engine-flutter-autoroll@skia.org Roll Dart SDK from 8531f7c2bdae to 59be21f25f2d (4 revisions) (flutter/flutter#183547)
2026-03-12 koji.wakamiya@gmail.com [ios][engine] Fix keyboard flicker when switching text fields (flutter/flutter#182661)
2026-03-11 34871572+gmackall@users.noreply.github.com Rename hcpp opt in flag to `enable-hcpp` (flutter/flutter#183151)
2026-03-11 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#183541)
2026-03-11 97480502+b-luk@users.noreply.github.com Re-enable gpu_test.dart for opengles, but disable specific tests in the file (flutter/flutter#183531)
2026-03-11 jacksongardner@google.com Use CLA-approved email and username for github workflow commits. (flutter/flutter#183540)
2026-03-11 okorohelijah@google.com [integration_test]UIScene Migration (flutter/flutter#182631)
2026-03-11 git@reb0.org ci: Run orchestrator for windows_arm_host_engine on Linux  (flutter/flutter#181075)
2026-03-11 mdebbar@google.com [web] Prevent Firefox auto-updates (flutter/flutter#183330)
2026-03-11 koji.wakamiya@gmail.com Add scrollCacheExtent to PageView (flutter/flutter#180411)
2026-03-11 danny@tuppeny.com Remove redundant VS Code code/tests (flutter/flutter#183506)
2026-03-11 danny@tuppeny.com Roll pub packages (flutter/flutter#183527)
2026-03-11 34871572+gmackall@users.noreply.github.com Reapply "Make HCPP upgrading work for vd/tlhc (#181024)" (#183310) (flutter/flutter#183360)
2026-03-11 victorsanniay@gmail.com Add awaits to dev/ callsites (flutter/flutter#183479)
2026-03-11 stuartmorgan@google.com Standardize review timeline guidance (flutter/flutter#183465)
2026-03-11 chingjun@google.com Rename ExecutionModelToString to ExecutionModelToStringName. (flutter/flutter#183461)
2026-03-11 34871572+gmackall@users.noreply.github.com Fix input mirroring in platform views when system language is RTL, and support is set in manifest. (flutter/flutter#183472)
2026-03-11 engine-flutter-autoroll@skia.org Roll Packages from ee460d6 to ecace66 (10 revisions) (flutter/flutter#183517)

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

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

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants