-
Notifications
You must be signed in to change notification settings - Fork 77
Fixes for #pragma warning
#613
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
Conversation
…from `int` to `void`
…ragma warning push`
…tting clobbered after the use of `preproc_expr()`
It seems that you're completely missing the point on how Firstly, Secondly, if you thought #pragma warning disable 203
#pragma warning pop
new var;
main(){} In this example the compiler wouldn't warn about Thirdly, usually an unmatched |
TBH I was unsure about some of these things, but |
Nothing wrong with complex sets of conditional pushes/pops, as long as they match up in the end. |
If you don't want to pop, just don't push. Disabling a warning doesn't require a push first. |
But that code is probably a mistake. Maybe it should be a warning instead, but still. The #pragma warning pop
#pragma warning pop
#pragma warning pop To restore all three. |
Well, as I already mentioned in #610, the main reason I made them as errors was because I wanted unmatched |
What this PR does / why we need it:
#pragma warning disable
not being reset at the end of each compilation pass, resulting in its effects being carried onto the subsequent compilation passes (see #pragma warning #610).#pragma warning push
is used more times than#pragma warning pop
and vice versa (see #pragma warning #610).pc_pushwarnings()
andpc_popwarnings()
fromint
tovoid
.pc_pushwarnings()
never returnsFALSE
; in case of memory allocation failure it callserror(103)
, which aborts execution (as 103 is a fatal error), making the linereturn FALSE;
unreachable.Also, the values returned by both
pc_pushwarnings()
andpc_popwarnings()
are never used.#pragma warning push/pop/enable/disable
with tests (previously there weren't any).Which issue(s) this PR fixes:
Fixes #610
What kind of pull this is:
Additional Documentation: