Skip to content
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

Ignore warnings related to deprecated stdext::checked_array_iterator on MSVC (again) #3907

Merged
merged 10 commits into from
Sep 17, 2024

Conversation

nickbianco
Copy link
Member

@nickbianco nickbianco commented Sep 13, 2024

Brief summary of changes

Update to the fix implemented in #3895.

Testing I've completed

Looking for feedback on...

CHANGELOG.md (choose one)

  • no need to update because...internal build system fix.

This change is Reviewable

@nickbianco nickbianco removed the request for review from adamkewley September 13, 2024 23:26
@adamkewley
Copy link
Contributor

adamkewley commented Sep 16, 2024

Looking into this a little, because I kept having issues with it downstream, the problem (iirc) is also be that the latest MSVC can't build OpenSim's dependencies/ either, due to the same warning. It wasn't caught in the previous PR because CI's caching the dependency build.

I ended up having to read into this particular deprecation more, and it turns out that MSVC has a very specific flag for disabling it:

  • /D_SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING

Which is probably better to use than the /wd flag, because it is specific to this warning. In my downstream build, I'm abusing the fact that CXXFLAGS can be set as an environment variable at the top-level before building Opensim's dependencies:

https://github.com/ComputationalBiomechanicsLab/opensim-creator/blob/0.5.14/scripts/build_windows.py#L102

I wouldn't recommend deploying it this way (e.g. by telling users to set some environment variable) in opensim-core. The solution is probably going to be to use target_compile_definitions when configuring OpenSim's targets to add the flag, and then to separately update dependencies/CMakeLists.txt:197 by explicitly passing the flag via CMAKE_CXX_ARGS or similar to the spdlog build. I'm away for a few days so I can't handle this right now, but that's how I'd tackle it.

Or, even better, de-integrate spdlog+fmt and revert to something that uses snprintf etc., or wait a year for C++20 with <format> support to roll around to all target platforms :>

@nickbianco
Copy link
Member Author

Thanks @adamkewley, hopefully that does the trick.

wait a year for C++20 with support to roll around to all target platforms :>

I think this is the move. fmt and spdlog are pretty heavily integrated via the logging system currently.

@adamkewley
Copy link
Contributor

Seems to have done trick! Nice work @nickbianco !

@adamkewley adamkewley merged commit 292147c into main Sep 17, 2024
11 checks passed
@nickbianco nickbianco deleted the silence_checked_array_iterator_warning branch September 17, 2024 16:31
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