Rename ExecutionModelToString to ExecutionModelToStringName.#183461
Conversation
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.
|
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. |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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 ...
}
} // namespaceReferences
- 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
staticfor file-local functions. (link)
|
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. |
|
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. |
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
…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
…#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
…#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
…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
The function of the same name
ExecutionModelToStringwas 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.