export proper syntax for GSL_SUPPRESS for new VS #1213
Conversation
carsonRadtke
added
Priority: High
github-project-automation
bot
moved this to Open
in Microsoft/GSL Pull Requests In-Flight
carsonRadtke
moved this from Open
to Ready for Review
in Microsoft/GSL Pull Requests In-Flight
github-project-automation
bot
moved this from Ready for Review
to Approved
in Microsoft/GSL Pull Requests In-Flight
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.
carsonRadtke
force-pushed
the
carsonradtke/suppress-c4875
branch
from
7e1ee6b to
1056583
Compare
github-project-automation
bot
moved this from Approved
to Merged
in Microsoft/GSL Pull Requests In-Flight
| // 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 <[email protected]>
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 <[email protected]>
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.