Skip to content

MSVC build fix: include <string> instead of <string_view> #1192

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
wants to merge 1 commit into from
Closed

MSVC build fix: include <string> instead of <string_view> #1192

wants to merge 1 commit into from

Conversation

vorlac
Copy link

@vorlac vorlac commented Aug 16, 2024

  • Fixes some unrecognized std::string usage after the 17.11 update of VS2022, since the changes bring an STL update where the <string_view> header no longer includes <xstring> by default.

* Fixes some unrecognized std::string usage after the 17.11 update of VS2022, since the changes bring an STL update where the <string_view> header no longer includes <xstring> by default.
@vorlac
Copy link
Author

vorlac commented Aug 16, 2024

After updating VS2022 earlier today I noticed this new break. It seems to be caused by these recent changes to the Microsoft STL: https://github.com/microsoft/STL/blame/b191409cec71aa837072a5d1ebafb041989c71dd/stl/inc/string_view#L14-L16

The error occurs when BSONCXX_POLY_USE_STD is set. I'm not sure if just changing the include here is the preferred approach, but I figured I'd start with this. I can update the PR with any alternate/preferred approaches if you'd prefer avoiding the include of <string> to get around this.

This can also revert back to the older behavior by defining: _LEGACY_CODE_ASSUMES_STRING_VIEW_INCLUDES_XSTRING

The exact errors this led to:

[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\include\mongocxx\v_noabi\mongocxx/events/heartbeat_failed_event.hpp(56): error C2039: 'string': is not a member of 'std'
[build] C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.41.34120\include\__msvc_string_view.hpp(22): note: see declaration of 'std'
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\include\mongocxx\v_noabi\mongocxx/events/heartbeat_failed_event.hpp(56): error C3646: 'message': unknown override specifier
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\include\mongocxx\v_noabi\mongocxx/events/heartbeat_failed_event.hpp(56): error C2059: syntax error: '('
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\include\mongocxx\v_noabi\mongocxx/events/heartbeat_failed_event.hpp(56): error C2238: unexpected token(s) preceding ';'
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\lib\mongocxx\v_noabi\mongocxx\events\heartbeat_failed_event.cpp(28): error C2039: 'message': is not a member of 'mongocxx::v_noabi::events::heartbeat_failed_event'
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\include\mongocxx\v_noabi\mongocxx/events/heartbeat_failed_event.hpp(36): note: see declaration of 'mongocxx::v_noabi::events::heartbeat_failed_event'
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\lib\mongocxx\v_noabi\mongocxx\events\heartbeat_failed_event.cpp(28): error C2270: 'message': modifiers not allowed on nonmember functions
[build] C:\Users\sal\development\vorlac\routeanalytics-cuda\extern\mongo-cxx-driver\src\mongocxx\lib\mongocxx\v_noabi\mongocxx\events\heartbeat_failed_event.cpp(30): error C2065: '_failed_event': undeclared identifier

VS2022 installation details:

sal:zephyrus • C:\Program Files (x86)\Microsoft Visual Studio\Installer
└>  .\vswhere.exe
Visual Studio Locator version 3.1.7+f39851e70f [query version 3.11.2133.16870]
Copyright (C) Microsoft Corporation. All rights reserved.

instanceId: 656b9610
installDate: 5/8/2024 11:09:22 PM
installationName: VisualStudio/17.11.0+35208.52
installationPath: C:\Program Files\Microsoft Visual Studio\2022\Community
installationVersion: 17.11.35208.52
productId: Microsoft.VisualStudio.Product.Community
productPath: C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\IDE\devenv.exe
state: 4294967295
isComplete: 1
isLaunchable: 1
isPrerelease: 0
isRebootRequired: 0
displayName: Visual Studio Community 2022
description: Powerful IDE, free for students, open-source contributors, and individuals
channelId: VisualStudio.17.Release
channelUri: https://aka.ms/vs/17/release/channel
enginePath: C:\Program Files (x86)\Microsoft Visual Studio\Installer\resources\app\ServiceHub\Services\Microsoft.VisualStudio.Setup.Service
installedChannelId: VisualStudio.17.Release
installedChannelUri: https://aka.ms/vs/17/release/channel
releaseNotes: https://docs.microsoft.com/en-us/visualstudio/releases/2022/release-notes-v17.11#17.11.0
resolvedInstallationPath: C:\Program Files\Microsoft Visual Studio\2022\Community
thirdPartyNotices: https://go.microsoft.com/fwlink/?LinkId=661288
updateDate: 2024-08-15T19:11:51.0986827Z
catalog_buildBranch: d17.11
catalog_buildVersion: 17.11.35208.52
catalog_id: VisualStudio/17.11.0+35208.52
catalog_localBuild: build-lab
catalog_manifestName: VisualStudio
catalog_manifestType: installer
catalog_productDisplayVersion: 17.11.0
catalog_productLine: Dev17
catalog_productLineVersion: 2022
catalog_productMilestone: RTW
catalog_productMilestoneIsPreRelease: False
catalog_productName: Visual Studio
catalog_productPatchVersion: 0
catalog_productPreReleaseMilestoneSuffix: 8.0
catalog_productSemanticVersion: 17.11.0+35208.52
catalog_requiredEngineVersion: 3.11.2177.7163
properties_campaignId: 3651:1b4ee2a2bf174e818eb95a977ef93bd3
properties_channelManifestId: VisualStudio.17.Release/17.11.0+35208.52
properties_nickname: 
properties_setupEngineFilePath: C:\Program Files (x86)\Microsoft Visual Studio\Installer\setup.exe

@vorlac vorlac changed the title include <string> instead of <string_view> MSVC build fix: include <string> instead of <string_view> Aug 16, 2024
@eramongodb
Copy link
Contributor

eramongodb commented Aug 16, 2024

@vorlac Thank you for submitting this pull request!

This is an Include What You Use (IWYU) violation by the C++ Driver being exposed by the STL implementation of the standard library. <string_view> is not specified to include <string>. The correct fix is to include <string> wherever std::string is used in accordance with IWYU.

Filed CXX-3089 to track this issue. Please review the proposed changes in #1193 to validate it fixes the build failures observed in your environment.

@eramongodb eramongodb closed this Aug 16, 2024
@vorlac
Copy link
Author

vorlac commented Aug 16, 2024

Sure thing, happy to contribute however I can for projects like this one. OK nice, sounds good. The way IWYU was treating this include makes perfect sense. I'll take a look at the issue you linked when I get done work in a few hours just to confirm it all builds without issues in my environment.

What should I use to properly test as much coverage as possible for something like this? Is it sufficient to build all of the tests bundled in this repo, or would you recommend a larger open source 3rd party project that uses this library instead? The project I'm working on now is still in the very early stages of me just familiarizing myself with this library and the functionality it offers so it most likely isn't going to pull in too many headers to build... at least for another few weeks probably.

Also, if you just want to continue the conversation in the new issue you opened, feel free to just respond to this directly in there and I'll see it when I free up to confirm the patch for you in a few hours.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants