Consider adding a test with /analyze:plugin EspXEngine.dll
#2094
Labels
test
Related to test code
Comments
CaseyCarter
added
decision needed
|
My gut reaction is that the resulting signal-to-noise ratio would be extremely low and that this wouldn't be worthwhile. That said, I wouldn't mind someone doing the investigation to determine if there's a reasonable compromise subset of the Core Guidelines checkers that we could enable without spamming suppression fluff everywhere. (I suspect that, e.g., "Use |
|
Possibly it can be done like this: #pragma warning(disable: ALL_CPPCORECHECK_WARNINGS)
#pragma warning(default: 26433 26435 ...) |
StephanTLavavej
changed the title
/analyze:plugin EspXEngine.dll
StephanTLavavej
added
test
|
We talked about this in our weekly maintainer meeting - we think that adding an "include all headers" test, covering specific CppCoreCheck warnings (as you mentioned immediately above), could be useful. As I mentioned in Discord, the STL's build is less useful to provide such coverage as it compiles only a fraction of the STL (primarily iostreams/filesystem/multithreading). We definitely don't want to adopt all C++ Core Guidelines for this codebase, so we'll need to select exactly which warnings to enable. Starting with the "avoid redundant |
|
...yes and... the sideaffect of this STL codebase when used on MSVC (vs2022) code analyzer generates a LOT of noise in very simple programs. A 29 line c++ program that includes cstdint, optional and stdexcept results in 67 lines of warnings all from this STL. Naturally that level of warnings overwhelms useful diagnostics from the compiler. The warnings are all from MSVC analyzer's A partnership between STL and MSVC is likely needed to consider how to resolve the excessive noise.
#include <cstdint>
#include <optional>
#include <stdexcept>
int gds = 7;
int sdi = 2;
bool array1[5] = { true, false, true, false, true };
void bugrepro(std::optional<int32_t> candidate = std::nullopt);
void bugrepro(std::optional<int32_t> candidate) {
if (candidate.has_value()) {
if (candidate < 0 || candidate > gds)
throw std::out_of_range("bad value");
}
else {
if (sdi <= 0)
return;
candidate = sdi;
}
if (!array1[candidate.value()]) {
throw std::runtime_error("no good");
}
}
int main() {
bugrepro();
}compiled with results in |
See also #3081. It's a bit suprising to me that the exception specification of |
|
To scope the size of STL analyzer warnings, I have 10+ thousands of warnings from STL while compiling my app with the VC's analyzer as above. I uses a tiny fraction of STL so there are far more STL analyzer warnings present that I haven't seen. Some warnings are invalid like @StephanTLavavej wrote "use gsl but gsl needs stl circular". Others are more curious like I am able to silence analyzer warnings in my specific app's STL usage with add_compile_definitions(
_STL_EXTRA_DISABLED_WARNINGS=26485\ 26432\ 26490\ 26455\ 26492\ 26481\ 26473\ 26472\ 26462\ 26440\ 26474\ 26457\ 26494\ 26800\ 26496\ 26447\ 26818\ 26460\ 26409\ 26436\ 26408\ 26401\ 26497\ 26496\ 26461\ 26475\ 26403\ 26465\ 26445\ 26491\ 26411\ 26495\ 26493\ 26467\ 26415\ 26418\ 26459\ 26439\ 26417\ 26437\ 26400\ 26466\ 26820\ 26416
)Not directly related to STL...is that MSVC distributes many other headers (like |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There's already
/analyzeoption:STL/azure-devops/cmake-configure-build.yml
Line 41 in cc515bb
/analyze:plugin EspXEngine.dllcould be added to add C++ Core Guidelines check.See more: Use the C++ Core Guidelines checkers
As an example #2069 changes can be enforced: https://godbolt.org/z/jfx9dzb1T
It may produce too many warnings that won't b addressed though. These can be suppressed either by ordinary suppression or by
[[gsl::suppress(...)]]attribute.The text was updated successfully, but these errors were encountered: