gh-135183: Suppress MSVC warning 5274 locally

[encukou]

• Issue: MSVC 5274 warning suppression for _Py_ALIGN_AS should be local #135183

[zooba]

Why? The suppression was restricted to our headers, which is local enough. It's only warning about changed behaviour from the old syntax, which we never used.

Are we going to replicate the same suppression code every time it gets used?

[encukou]

We want core devs to use _Py_ALIGN_AS, and when they do, they need to think about this issue (in cases where the warning shows up).

It's only warning about changed behaviour from the old syntax, which we never used.

AFAICS, it's not about syntax changes. MSVC 17.9 vs. older versions produce different ABI for _Alignas.

[encukou]

It might be clearer with the example from the MS docs:

struct Outer
{
    _Alignas(32) struct Inner { int i; } member1;
    struct Inner member2;
};

The layout of this structure is different depending on what version of MSVC you compile with. We do not want to define such a struct in our headers.

Are we going to replicate the same suppression code every time it gets used?

Yes.

[zooba]

We do not want to define such a struct in our headers.

Then suppressing the warning isn't the way, and we should be removing/changing _Py_ALIGN_AS? Or blocking earlier MSVC versions in the headers?

[encukou]

Then suppressing the warning isn't the way, and we should be removing/changing _Py_ALIGN_AS?

I think suppressing is the way. Once you (as a core dev) confirm that you're not defining such a struct, add the suppression.
The warning is there to make you consider this weird compiler bug; you wouldn't do that otherwise.

Hm, maybe I wasn't clear here:

Are we going to replicate the same suppression code every time it gets used?

Yes.

By “it” I mean the suppression code, not _Py_ALIGN_AS.
The suppression is only needed when defining a struct and a member at once.

... which suggests another possible workaround here: define the struct for PyASCIIObject.state outside of PyASCIIObject. Then no suppression is necessary -- but that means we don't get to add an example of what to do when you get C5274 from the CI.

Or blocking earlier MSVC versions in the headers?

Well, you're the expert on that :)
I feel that 17.8 should be supported, and it's worth it to work around its bug.

[encukou]

See also #135209, for a possible bigger change to _Py_ALIGN_AS.

[zooba]

I like the other change better. Including the type and the alignment as arguments to the macro should be easier to support long-term.