[PATCH] fix for C4141 warning on MSVC

Started by Michail Nikolaevabout 8 years ago5 messageshackers
Jump to latest
#1Michail Nikolaev
michail.nikolaev@gmail.com

Attachments:

C4141.patchapplication/octet-stream; name=C4141.patchDownload+8-8
#2Thomas Munro
thomas.munro@gmail.com
In reply to: Michail Nikolaev (#1)
Re: [PATCH] fix for C4141 warning on MSVC

On Wed, Jan 24, 2018 at 1:16 PM, Michail Nikolaev
<michail.nikolaev@gmail.com> wrote:

Just very small fix for C4141 warning
(https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4141).

Also could be viewed on Github -
https://github.com/michail-nikolaev/postgres/commit/38a590a00110a4ea870d625470e4c898e5ad79aa

Tested both MSVC and gcc builds.

Thanks. This is similar to the fix I proposed over here:

/messages/by-id/CAEepm=2iTKvbebiK3CdoczQk4_FfDt1EeU4c+nGE340JH7gQ0g@mail.gmail.com

Two differences:

1. I also fixed a warning from antique GCC which didn't understand this magic.
2. You also defined it as plain old "inline" for unknown compilers.

Perhaps we should combine these?

--
Thomas Munro
http://www.enterprisedb.com

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#2)
Re: [PATCH] fix for C4141 warning on MSVC

Thomas Munro <thomas.munro@enterprisedb.com> writes:

On Wed, Jan 24, 2018 at 1:16 PM, Michail Nikolaev
<michail.nikolaev@gmail.com> wrote:

Just very small fix for C4141 warning

Thanks. This is similar to the fix I proposed over here:
/messages/by-id/CAEepm=2iTKvbebiK3CdoczQk4_FfDt1EeU4c+nGE340JH7gQ0g@mail.gmail.com

Perhaps we should combine these?

+1. The previous thread seems to have died off with nothing happening,
but we still have those issues to resolve. I like the idea of making
the macro be a direct drop-in substitute for "inline" rather than
something you use in addition to "inline". And if we do that, it
definitely should expand to plain "inline" if we have no other idea.

regards, tom lane

#4Thomas Munro
thomas.munro@gmail.com
In reply to: Tom Lane (#3)
Re: [PATCH] fix for C4141 warning on MSVC

On Wed, Jan 24, 2018 at 1:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Munro <thomas.munro@enterprisedb.com> writes:

On Wed, Jan 24, 2018 at 1:16 PM, Michail Nikolaev
<michail.nikolaev@gmail.com> wrote:

Just very small fix for C4141 warning

Thanks. This is similar to the fix I proposed over here:
/messages/by-id/CAEepm=2iTKvbebiK3CdoczQk4_FfDt1EeU4c+nGE340JH7gQ0g@mail.gmail.com

Perhaps we should combine these?

+1. The previous thread seems to have died off with nothing happening,
but we still have those issues to resolve. I like the idea of making
the macro be a direct drop-in substitute for "inline" rather than
something you use in addition to "inline". And if we do that, it
definitely should expand to plain "inline" if we have no other idea.

Here's one like that.

--
Thomas Munro
http://www.enterprisedb.com

Attachments:

fix-warnings-about-always-inline-v2.patchapplication/octet-stream; name=fix-warnings-about-always-inline-v2.patchDownload+4-6
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#4)
Re: [PATCH] fix for C4141 warning on MSVC

Thomas Munro <thomas.munro@enterprisedb.com> writes:

Here's one like that.

Pushed; we'll soon see if the buildfarm likes it. I added a tweak to
prevent forced inlining at -O0, as discussed in the other thread;
and worked on the comments a bit.

regards, tom lane