Skip to content

FlutterDesktopEngineCreate takes argument by reference #75465

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

Closed
knopp opened this issue Feb 5, 2021 · 2 comments · Fixed by flutter/engine#25439
Closed

FlutterDesktopEngineCreate takes argument by reference #75465

knopp opened this issue Feb 5, 2021 · 2 comments · Fixed by flutter/engine#25439
Labels
a: desktop Running on desktop engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-windows Building on or for Windows specifically

Comments

@knopp
Copy link
Member

knopp commented Feb 5, 2021

I'm not sure how much of a problem this in practice, but I'm bringing this up just in case.

The methods in flutter_windows.h are declared as extern "C", but FlutterDesktopEngineCreate takes engine_properties by const reference, which is not a C thing.

Right now calling it with a pointer (i.e. through FFI or from Rust) works, and I don't suppose it's going to change in future, but it seems like it's relying on unspecified behavior.

@knopp knopp added platform-windows Building on or for Windows specifically a: desktop Running on desktop labels Feb 5, 2021
@knopp knopp changed the title Windows: FlutterDesktopEngineCreate takes argument by reference FlutterDesktopEngineCreate takes argument by reference Feb 5, 2021
@stuartmorgan-g
Copy link
Contributor

That should definitely be fixed, the sooner the better to minimize possible breaks (in practice, probably nobody will be broken because the template only uses the wrapper). It's there because for some reason I'm incapable of remembering that references are C++, not C.

/cc @cbracken

@gspencergoog gspencergoog added the P2 Important issues not at the top of the work list label Feb 5, 2021
@pedromassangocode pedromassangocode added engine flutter/engine repository. See also e: labels. passed first triage labels Feb 9, 2021
cbracken added a commit to cbracken/flutter_engine that referenced this issue Apr 6, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
cbracken added a commit to cbracken/flutter_engine that referenced this issue Apr 6, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
cbracken added a commit to cbracken/flutter_engine that referenced this issue Apr 6, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
cbracken added a commit to cbracken/flutter_engine that referenced this issue Apr 6, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
cbracken added a commit to flutter/engine that referenced this issue Apr 6, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
duanqz pushed a commit to duanqz/engine that referenced this issue Apr 16, 2021
FlutterDesktopEngineCreate is part of our C API. We were using a C++
reference type instead of a C-compatible pointer type.

This is a breaking change to anyone calling this directly; we believe
this should affect few people because the Windows template only uses the
`FlutterEngine` wrapper in
`shell/platform/windows/client_wrapper/include/flutter/flutter_engine.h`.

Fixes flutter/flutter#75465
@github-actions
Copy link

github-actions bot commented Aug 3, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop engine flutter/engine repository. See also e: labels. P2 Important issues not at the top of the work list platform-windows Building on or for Windows specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants