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

Conversation

@bryanoltman
Copy link
Contributor

@bryanoltman bryanoltman commented Dec 11, 2024

GetLastError returns an unsigned 32 bit integer that was being implicitly cast to an int for the std::variant<..., int>. This was causing my build to fail with:

../../flutter/shell/platform/windows/platform_handler.cc(178,12): error: no viable conversion from returned value of type 'DWORD' (aka 'unsigned long') to function return type 'std::variant<std::wstring, int>' (aka 'variant<basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t>>, int>')
  178 |     return ::GetLastError();
      |            ^~~~~~~~~~~~~~~~
../../../../../../../Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.42.34433/include\variant(923,7): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'DWORD' (aka 'unsigned long') to 'const variant<basic_string<wchar_t>, int> &' for 1st argument
  923 | class variant : private _SMF_control<_Variant_destroy_layer<_Types...>, _Types...> { // discriminated union
      |       ^~~~~~~

Commands:

 ./flutter/tools/gn --runtime-mode release --no-rbe
ninja -C .\out\host_release windows gen_snapshot flutter/build/archives:windows_flutter

Explicitly casting ::GetLastError to an int fixes this issue.

I'm running on Windows 11 (Version 10.0.26100 Build 26100) with VS 2022 Community Edition.

@loic-sharma

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

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. 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.

Copy link
Member

@loic-sharma loic-sharma left a comment

Choose a reason for hiding this comment

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

LGTM

@loic-sharma
Copy link
Member

I requested a test exemption.

@eseidel
Copy link
Contributor

eseidel commented Dec 11, 2024

@bryanoltman do you have merge permission or did you want Loic or I to press ze button?

@bryanoltman
Copy link
Contributor Author

@eseidel I do not have permissions, so be my guest 😄

@eseidel
Copy link
Contributor

eseidel commented Dec 12, 2024

Checks have passed and you have lgtm, so merging. Hopefully I'm still doing this correctly. 🤞

@eseidel eseidel merged commit 847deb2 into flutter:main Dec 12, 2024
28 checks passed
@eseidel eseidel deleted the bo/fix-windows-compile branch December 12, 2024 01:23
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 12, 2024
bryanoltman added a commit to shorebirdtech/engine that referenced this pull request Dec 12, 2024
`GetLastError` returns an unsigned 32 bit integer that was being
implicitly cast to an int for the std::variant<..., int>. This was
causing my build to fail with:

```
../../flutter/shell/platform/windows/platform_handler.cc(178,12): error: no viable conversion from returned value of type 'DWORD' (aka 'unsigned long') to function return type 'std::variant<std::wstring, int>' (aka 'variant<basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t>>, int>')
  178 |     return ::GetLastError();
      |            ^~~~~~~~~~~~~~~~
../../../../../../../Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.42.34433/include\variant(923,7): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'DWORD' (aka 'unsigned long') to 'const variant<basic_string<wchar_t>, int> &' for 1st argument
  923 | class variant : private _SMF_control<_Variant_destroy_layer<_Types...>, _Types...> { // discriminated union
      |       ^~~~~~~
```

Commands:

```
 ./flutter/tools/gn --runtime-mode release --no-rbe
ninja -C .\out\host_release windows gen_snapshot flutter/build/archives:windows_flutter
```

Explicitly casting `::GetLastError` to an int fixes this issue.

I'm running on Windows 11 (Version 10.0.26100 Build 26100) with VS 2022
Community Edition.

@loic-sharma

Co-authored-by: Eric Seidel <eric@shorebird.dev>
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Dec 12, 2024
…160134)

flutter/engine@92de3d0...847deb2

2024-12-12 bryanoltman@gmail.com Cast ::GetLastError to int
(flutter/engine#57113)
2024-12-12 jonahwilliams@google.com Manual Skia roll from 79a7b95e32fe
to 0aec6f7bfbc8 (flutter/engine#57134)

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 jonahwilliams@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://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
@stuartmorgan-g
Copy link
Contributor

test-exempt: code refactor with no semantic change

@eseidel Please don't land PRs that need a test exemption without them having a test exemption.

@eseidel
Copy link
Contributor

eseidel commented Dec 12, 2024

Apologies, my bad. Checks passed so I assumed it was clear. (I assumed something like that would have been a check.)

nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
`GetLastError` returns an unsigned 32 bit integer that was being
implicitly cast to an int for the std::variant<..., int>. This was
causing my build to fail with:

```
../../flutter/shell/platform/windows/platform_handler.cc(178,12): error: no viable conversion from returned value of type 'DWORD' (aka 'unsigned long') to function return type 'std::variant<std::wstring, int>' (aka 'variant<basic_string<wchar_t, char_traits<wchar_t>, allocator<wchar_t>>, int>')
  178 |     return ::GetLastError();
      |            ^~~~~~~~~~~~~~~~
../../../../../../../Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.42.34433/include\variant(923,7): note: candidate constructor (the implicit copy constructor) not viable: no known conversion from 'DWORD' (aka 'unsigned long') to 'const variant<basic_string<wchar_t>, int> &' for 1st argument
  923 | class variant : private _SMF_control<_Variant_destroy_layer<_Types...>, _Types...> { // discriminated union
      |       ^~~~~~~
```

Commands:

```
 ./flutter/tools/gn --runtime-mode release --no-rbe
ninja -C .\out\host_release windows gen_snapshot flutter/build/archives:windows_flutter
```

Explicitly casting `::GetLastError` to an int fixes this issue.

I'm running on Windows 11 (Version 10.0.26100 Build 26100) with VS 2022
Community Edition.

@loic-sharma

Co-authored-by: Eric Seidel <eric@shorebird.dev>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants