export proper syntax for GSL_SUPPRESS for new VS#1213
Conversation
A new Visual Studio version will soon be available that deprecates the
old syntax for gsl::suppress. Customers will now get a C4875 diagnostic
on suppressions that look like `gsl::suppress(x)` urging them to use
`gsl::suppress("x")` instead.
This change updates the `GSL_SUPPRESS` macro to preprocess
GSL_SUPPRESS(x) to gsl::suppress("x") on clang and new versions of MSVC.
7e1ee6b to
1056583
Compare
| // Visual Studio versions after 2022 (_MSC_VER > 1944) support the justification message. | ||
| #define GSL_SUPPRESS(x) [[gsl::suppress(#x)]] | ||
| #elif defined(_MSC_VER) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) | ||
| #define GSL_SUPPRESS(x) [[gsl::suppress(#x)]] |
There was a problem hiding this comment.
The PR description says that the macro changed for new VS, it does not mention a change for old VS.
Also the documentation change in this PR says that
Older versions of MSVC (VS 2022 and earlier) only understand
[[gsl::suppress(tag)]]without the double quotes aroundtag.
Yet this here changes the behaviour for old VS.
But I am surprised to see that it works on VS 2022 on my installation. So the documentation and the code change leave me confused.
There was a problem hiding this comment.
Good catch, that's wrong; line 54 should be suppress(x).
As for why it works on VS 2022...
I suspect it "works" in the sense that the program compiles without errors, but the suppression will not actually be picked up by code analysis. This is because, for older compilers (pre 19.50 toolset), the gsl::suppress argument was just stored and never checked (it could be gsl::suppress(<literally-anything>)) then during code analysis, the argument is actually analyzed and if it is garbage, it would be silently thrown out.
The latest version of the compiler does a little better and the front-end will report some problems with a bad gsl::suppress, but it still won't report everything.
I'll create an issue to get the behavior fixed for old VS versions.
There was a problem hiding this comment.
Sorry, I was unprecise. I tested on VS2022 17.14.21, toolset v143.
I have a line of code that triggers a code analysis warning. Above that line I wrote GSL_SUPPRESS(foo).
With #define GSL_SUPPRESS(x) I get the code analysis warning. That is expected as there is no suppression active.
With #define GSL_SUPPRESS(x) [[gsl::suppress(x)]] the warning is suppressed.
With #define GSL_SUPPRESS(x) [[gsl::suppress(#x)]] the warning is also suppressed.
Maybe latest VS2022 already has some change that allows [[gsl::suppress(foo)]] and [[gsl::suppress("foo")]]?
There was a problem hiding this comment.
Ah, I see. I was misremembering the timing that things got merged.
For the VS2022 17.13 release (toolset v143, compiler 19.43), we added support for gsl::suppress("x") in addition to the existing gsl::suppress(x). Before that, the compiler silently ignored gsl::suppress("x"). Example: https://godbolt.org/z/jb6TG5Y6o
A new Visual Studio version will soon be available that deprecates the old syntax for gsl::suppress. Customers will now get a C4875 diagnostic on suppressions that look like gsl::suppress(x) urging them to use gsl::suppress("x") instead.
When I wrote this PR, C4875 was going to go out with VS2026 18.0 (toolset v150, compiler 19.50). Unfortunately, it was reverted at the last second and is now slotted to go out with the next compiler release. According to our internal tracking, it is part of toolset v151, which I guess will be made available for preview starting Q1 next year (https://devblogs.microsoft.com/cppblog/new-release-cadence-and-support-lifecycle-for-msvc-build-tools/
PR microsoft#1213 changed this line. According to https://github.com/microsoft/GSL/pull/1213/files#r2586073058 the change was unintended. This PR reverts the change to the previous implementation.
PR #1213 changed this line. According to https://github.com/microsoft/GSL/pull/1213/files#r2586073058 the change was unintended. This PR reverts the change to the previous implementation. Co-authored-by: Werner Henze <w.henze@avm.de>
A new Visual Studio version will soon be available that deprecates the
old syntax for gsl::suppress. Customers will now get a C4875 diagnostic
on suppressions that look like `gsl::suppress(x)` urging them to use
`gsl::suppress("x")` instead.
This change updates the `GSL_SUPPRESS` macro to preprocess
GSL_SUPPRESS(x) to gsl::suppress("x") on clang and new versions of MSVC.
PR #1213 changed this line. According to https://github.com/microsoft/GSL/pull/1213/files#r2586073058 the change was unintended. This PR reverts the change to the previous implementation. Co-authored-by: Werner Henze <w.henze@avm.de>
* export proper syntax for GSL_SUPPRESS for new VS (#1213) A new Visual Studio version will soon be available that deprecates the old syntax for gsl::suppress. Customers will now get a C4875 diagnostic on suppressions that look like `gsl::suppress(x)` urging them to use `gsl::suppress("x")` instead. This change updates the `GSL_SUPPRESS` macro to preprocess GSL_SUPPRESS(x) to gsl::suppress("x") on clang and new versions of MSVC. * Revert unintended change to GSL_SUPPRESS (#1226) PR #1213 changed this line. According to https://github.com/microsoft/GSL/pull/1213/files#r2586073058 the change was unintended. This PR reverts the change to the previous implementation. Co-authored-by: Werner Henze <w.henze@avm.de> --------- Co-authored-by: Werner Henze <34543625+beinhaerter@users.noreply.github.com> Co-authored-by: Werner Henze <w.henze@avm.de>
…28527) ### Description Upgrades `microsoft_gsl` to v4.2.1 and patches `GSL_SUPPRESS` to stringify its argument for MSVC, fixing the C4875 deprecation warning in VC++ 18.6.0. - **`cmake/deps.txt`** — Bump to v4.2.1 - **`cmake/patches/gsl/1064.patch`** — Removed; the NVCC guard (`!defined(__NVCC__)`) is already upstream in v4.2.1 - **`cmake/patches/gsl/1213.patch`** — New patch: `[[gsl::suppress(x)]]` → `[[gsl::suppress(#x)]]` for MSVC, matching upstream microsoft/GSL@543d0dd (PR microsoft/GSL#1213) - **`cmake/external/onnxruntime_external_deps.cmake`** — Removed the `if(onnxruntime_USE_CUDA)` conditional around the patch; the stringify fix applies to all MSVC builds, not just CUDA - **`cmake/vcpkg.json`** — Added `"version>=": "4.2.1"` constraint for `ms-gsl` to ensure vcpkg builds also pick up a version that includes the NVCC guard fix ### Motivation and Context GSL v4.0.0's `GSL_SUPPRESS` macro passes a non-string-literal to `[[gsl::suppress()]]`. VC++ 18.6.0 deprecates this form (C4875), and a future MSVC release will remove it entirely, breaking the build. The warning fires on essentially every translation unit via `capture.h` / `narrow.h`. The upstream fix (stringify via `#x`) hasn't shipped in a GSL release yet, so a local patch is still needed until a future GSL release includes it. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tianleiwu <30328909+tianleiwu@users.noreply.github.com> Co-authored-by: Tianlei Wu <tlwu@microsoft.com>
A new Visual Studio version will soon be available that deprecates the old syntax for gsl::suppress. Customers will now get a C4875 diagnostic on suppressions that look like
gsl::suppress(x)urging them to usegsl::suppress("x")instead.This change updates the
GSL_SUPPRESSmacro to preprocess GSL_SUPPRESS(x) to gsl::suppress("x") on clang and new versions of MSVC.