Re: Add last commit LSN to pg_last_committed_xact()

On Sat, Jan 29, 2022 at 02:51:32PM -0500, James Coleman wrote:
> Oh, thanks. I'd seen some discussion previously on the list about
> clang not supporting it, but that seems to have been incorrect. Also I
> didn't know about that compiler site -- that's really neat.
>
> Here's an updated patch series using that approach; the first patch
> can (and probably should be) committed separately/regardless to update
> the pg_attribute_aligned to be used in MSVC.

I don't have much an opinion on 0002, except that I am worried about
more data added to PGPROC that make it larger. Now, 0001 looks like a
hidden gem.

Based on the upstream docs, it looks that using __declspec(align(a))
is right:
https://docs.microsoft.com/en-us/cpp/cpp/align-cpp?view=msvc-170

Is __declspec available in Visual Studio 2013? I can see it in the
upstream docs for 2015, but I am not sure about 2013.

> /* This must match the corresponding code in c.h: */
> #if defined(__GNUC__) || defined(__SUNPRO_C) || defined(__IBMC__)
> #define pg_attribute_aligned(a) __attribute__((aligned(a)))
> +#elif defined(_MSC_VER)
> +#define pg_attribute_aligned(a) __declspec(align(a))
> #endif
> typedef __int128 int128a

This change in ./configure looks incorrect to me. Shouldn't the
change happen in c-compiler.m4 instead?

> +#if defined(_MSC_VER)
> +#define pg_attribute_aligned(a) __declspec(align(a))
> +#endif

This way of doing things is inconsistent with the surroundings. I
think that you should have an #elif for _MSC_VER to keep all the
definitions of pg_attribute_aligned(9 & friends in the same block.

This makes me wonder whether we would should introduce noreturn, as
of:
https://docs.microsoft.com/en-us/cpp/c-language/noreturn?view=msvc-140
--
Michael