Not a bug, but cleanup of a code analysis warning for Windows in include\wx\msw\init.h (Issue #26322)
Leo Violette
lviolette created an issue (wxWidgets/wxWidgets#26322)The current definition of WinMain:
`
extern "C" int WINAPI WinMain(HINSTANCE hInstance,
HINSTANCE hPrevInstance,
wxCmdLineArgType lpCmdLine,
int nCmdShow)
{
}
`
produces the following code analysis warning:
> warning C28251: Inconsistent annotation for 'WinMain': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.26100.0\um\winbase.h(1062). .
I believe this can be fixed by modifying the definition to this:
`
extern "C" int WINAPI WinMain(_In_ HINSTANCE hInstance,
_In_opt_ HINSTANCE hPrevInstance,
_In_ wxCmdLineArgType lpCmdLine,
_In_ int nCmdShow)
{
}
`
Thank you.
Leo V.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26322)There are so many analyzer warnings in wx headers that I'm not sure it's worth fixing this one without addressing all the others. But yes, we probably could add these annotations, MinGW-w64 seems to define these macros as well so it shouldn't be a problem to use them (and we could always define them as nothing if they're not already defined).
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Christopher Kwiatkowski
docentYT left a comment (wxWidgets/wxWidgets#26322)Duplicate of #18529
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26322)So can you confirm that the patch above fixes the problem for you? We could apply it if it does.
FWIW I don't even know when this appears, even if I choose "Build + IntelliSense" in the combobox in the "Error List" window I still don't see it when compiling the samples, this probably requires enabling "Code Analysis" or something like that?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Leo Violette
lviolette left a comment (wxWidgets/wxWidgets#26322)@vadz Yes, I see the issue with code analysis enabled for a given project (Go to project properties - Configuration Properties - Code Analysis - General. I have Enabled Microsoft Code Analysis and Enabled Clang-Tidy analysis enabled and see this warning. Especially if I have a cpp file that uses wxIMPLEMENT_APP in it.
I'm not sure which patch you are asking about. This might be due to my lack of experience with git.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26322)The "patch above" is the change you proposed, i.e.
diff --git a/include/wx/msw/init.h b/include/wx/msw/init.h index 8cacdb2c36..54541d29e2 100644 --- a/include/wx/msw/init.h +++ b/include/wx/msw/init.h @@ -47,10 +47,10 @@ extern WXDLLIMPEXP_CORE int int nCmdShow = SW_SHOWNORMAL); #define wxIMPLEMENT_WXWIN_MAIN \ - extern "C" int WINAPI WinMain(HINSTANCE hInstance, \ - HINSTANCE hPrevInstance, \ - wxCmdLineArgType lpCmdLine, \ - int nCmdShow) \ + extern "C" int WINAPI WinMain(_In_ HINSTANCE hInstance, \ + _In_opt_ HINSTANCE hPrevInstance, \ + _In_ wxCmdLineArgType lpCmdLine, \ + _In_ int nCmdShow) \ { \ wxDISABLE_DEBUG_SUPPORT(); \ \
If this is enough to solve the problem, we can apply it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Leo Violette
lviolette left a comment (wxWidgets/wxWidgets#26322)My apologies. Yes, this patch resolves the issue.
Thank you.
Leo.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26322)I'm not sure if _In_ etc are always defined in MinGW and I'm not sure it's a good idea to define them ourselves if they are not, so I've done something a bit different in the linked PR. Does this still work, i.e. avoids the warning?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Leo Violette
lviolette left a comment (wxWidgets/wxWidgets#26322)Hello. Yes, that latest patch does work. However, I'm not sure anything that elaborate is needed?
wrapwin.h brings in windows.h.
windows.h brings in windef.h.
windef.h brings in minwindef.h
and minwindef.h brings in specstrings.h, which brings in sal.h.
If you're building on Windows with MinGW and Clang, could be they'd also get sal.h unless there's msvc specific things going on. Which very well could be.
At any rate, I'm fine with the change you've made. It's explicit and should be guaranteed to work.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.