Add VS Code project config and fix CMakeLists.txt for Windows/MSVC - #2
Conversation
- Fix CMakeLists.txt: MSVC-compatible flags, remove duplicate main() issue, make Boost optional, each demo is a standalone executable - Add CMakePresets.json with MSVC + MinGW presets - Add .vscode/: c_cpp_properties.json, settings.json, tasks.json, launch.json, extensions.json Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
PR Summary by QodoMake CMake build MSVC-friendly and add VS Code + CMake Presets
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
There was a problem hiding this comment.
Code Review
This pull request introduces VS Code configuration files, CMake presets, and updates the CMakeLists.txt to support MSVC and Windows-based development. Feedback on these changes highlights several critical issues: compiling demo source files directly into test executables will cause duplicate main symbol linker errors, and relying on CMAKE_BUILD_TYPE for compiler flags is problematic with multi-configuration generators like Visual Studio. Additionally, the reviewer suggests using modern target-specific include directories, improving task portability by avoiding PowerShell-specific commands and shell chaining, applying Windows preprocessor definitions to MinGW builds, and removing hardcoded Windows SDK versions from VS Code settings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Use generator expressions for MSVC flags (fixes multi-config generators where CMAKE_BUILD_TYPE is unset at configure time) - Move WIN32 compile definitions to if(WIN32) so MinGW also gets NOMINMAX/WIN32_LEAN_AND_MEAN/_CRT_SECURE_NO_WARNINGS - Replace legacy include_directories with per-target target_include_directories PRIVATE on each executable - Add TESTING compile definition to test targets and wrap main() in #ifndef TESTING guards in each demo .cpp to prevent duplicate main linker errors when compiling into test executables - Fix tasks.json: replace && chain with cmake --build --preset (shell-agnostic, works in PowerShell 5.1 and cmd.exe) - Fix tasks.json Clean Build: replace Remove-Item with cmake -E rm -rf (portable, no shell dependency) - Remove hardcoded windowsSdkVersion from c_cpp_properties.json; CMake Tools extension auto-detects it Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Code Review by Qodo
1.
|
- CMakePresets.json: version was a string ('4'); must be JSON integer 4
per the CMake presets schema or tools refuse to load the file
- launch.json: add comment to market_data_receiver_demo config noting
it requires Boost and the target is skipped when Boost is absent
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Runs a complete paper trading session with: - Geometric Brownian motion market data generator (AAPL/MSFT/GOOGL/TSLA/NVDA) - Three strategies: MA Crossover, Mean Reversion, DQN-Lite (numpy NN) - Full P&L tracker (realized + unrealized), commission, slippage - 15% max drawdown kill switch - Session report: win rate, profit factor, Sharpe ratio, per-symbol P&L Requires only: numpy, pandas (already installed) Run: python paper_trading.py Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Why
The existing
CMakeLists.txtused GCC-only compiler flags (-O3,-march=native,-Wall, etc.) that fail silently or error out under MSVC. It also compiled every.cppfile (each containing its ownmain()) into a sharedtrading_corelibrary and as separate executables, causing duplicatemainlinker errors. There was also no VS Code configuration to provide IntelliSense, build tasks, or debugger support.What changed
CMakeLists.txt- fixed for MSVC/Windows:if(MSVC)/else()branches using proper MSVC options (/O2,/Od,/Zi,/W4)NOMINMAX,WIN32_LEAN_AND_MEAN,_CRT_SECURE_NO_WARNINGSdefinitions for Windows compatibilitytrading_corestatic library (it caused duplicatemain()linker errors); each demo is now a standalone executablemarket_data_receiver_demois skipped gracefully if Boost is not foundws2_32/mswsocklink deps for Boost.Asio on WindowsCMakePresets.json(new):.vscode/(new):c_cpp_properties.json- MSVC x64 IntelliSense, C++17, driven by CMake Toolssettings.json- sets CMake build dir, generator, and C++ standardtasks.json- build tasks wired to CMake presets (default build on Ctrl+Shift+B)launch.json- debug configurations for all four demo executables (F5)extensions.json- recommends ms-vscode.cpptools and ms-vscode.cmake-toolsTrade-offs / notes
market_data_receiver_demotarget requires Boost 1.75+. Without it, the target is skipped and a CMake warning is printed - nothing else breaks.