build: Add support building with msvc#59
Open
lygstate wants to merge 10 commits into
Open
Conversation
There was a problem hiding this comment.
5 issues found across 45 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/engine/hal/simulator/simulator.cpp">
<violation number="1" location="src/engine/hal/simulator/simulator.cpp:3">
P1: Wrapping `LV_IMG_DECLARE(mouse_cursor_icon)` in `extern "C"` creates a C-linkage reference, but `mouse_cursor_icon.c` is compiled with the C++ compiler in this build, so the definition will not match and linking can fail.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+3
to
+7
| extern "C" { | ||
|
|
||
| LV_IMG_DECLARE(mouse_cursor_icon); /*Declare the image file.*/ | ||
|
|
||
| } |
There was a problem hiding this comment.
P1: Wrapping LV_IMG_DECLARE(mouse_cursor_icon) in extern "C" creates a C-linkage reference, but mouse_cursor_icon.c is compiled with the C++ compiler in this build, so the definition will not match and linking can fail.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/engine/hal/simulator/simulator.cpp, line 3:
<comment>Wrapping `LV_IMG_DECLARE(mouse_cursor_icon)` in `extern "C"` creates a C-linkage reference, but `mouse_cursor_icon.c` is compiled with the C++ compiler in this build, so the definition will not match and linking can fail.</comment>
<file context>
@@ -1,5 +1,11 @@
#include "./simulator.hpp"
+extern "C" {
+
+LV_IMG_DECLARE(mouse_cursor_icon); /*Declare the image file.*/
</file context>
Suggested change
| extern "C" { | |
| LV_IMG_DECLARE(mouse_cursor_icon); /*Declare the image file.*/ | |
| } | |
| LV_IMG_DECLARE(mouse_cursor_icon); /*Declare the image file.*/ |
Author
There was a problem hiding this comment.
You were wrong about mouse_cursor_icon.c, the compiler for mouse_cursor_icon.c is default to c, that's why I use
extern "C"
Author
|
I create a fork https://github.com/lvgljs/lvgljs to get this merged |
Bump @txikijs/types in package.json
Several JS-callable component methods guarded with a smaller argc than the highest argv[] index they read. Since these functions are registered with declared length 0, QuickJS does not pad argv, so passing too few arguments caused out-of-bounds reads of the argument array. Align each argc check with the highest argv index accessed.
Set the TJS_CFUNC_DEF length field to each method's real arity so QuickJS pads argv with undefined up to that count. This provides a defensive backstop against out-of-bounds argv reads on top of the argc guards, and reports correct Function.prototype.length. Scoped to methods that read beyond argv[0].
…riable] [build] /mnt/c/work/lvgl/lv_binding_js/deps/txiki/deps/libwebsockets/lib/system/async-dns/async-dns.c:801:27: error: variable ‘c’ set but not used [-Werror=unused-but-set-variable] [build] 801 | lws_adns_cache_t *c; [build] | ^ [build] cc1: all warnings being treated as errors
MSVC do not support for zero length array: Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
vcpkg install:
Summary by cubic
Add MSVC build support for Windows (simulator +
vcpkg), upgradetxikito v26.5.0, and harden QuickJS bindings to prevent argument OOB reads. Also improves QuickJS and Windows compatibility for smoother cross‑platform builds.New Features
WIN32_LEAN_AND_MEAN; newBUILD_LVGL_SIMULATOR=ONflag with Makefile updated; non‑MSVC simulator builds relax-Werrorfor unused-but-set variables.src/render/native/lv_bindings_js.h; use single‑argJS_IsArray; boolean literals and event handlers returnJS_UNDEFINED; accurate argc guards and declared function arities for native methods to avoid OOB argv reads and report correct lengths; renameArcto avoid a Windows name clash; remove zero‑lengthComponentClassFuncsarrays; initializeis_newin style handling.txikito v26.5.0 and@txikijs/typesto^26.5.0.Migration
vcpkg:.\vcpkg.exe install curl zlib libffi sdl2 libffi:x64-windows-static-DBUILD_LVGL_SIMULATOR=ON -DUSE_EXTERNAL_FFI=ON -DCMAKE_PREFIX_PATH=C:/work/vcpkg/packages/zlib_x64-windows;C:/work/vcpkg/packages/curl_x64-windows;C:/work/vcpkg/packages/libffi_x64-windows;C:/work/vcpkg/packages/sdl2_x64-windows -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded$<$<CONFIG:Debug>:Debug>Written for commit 34ffc37. Summary will update on new commits.
Closes: #57