1428664 - Could small-enough Maybe's be used as function parameters?

Status: RESOLVED WONTFIX

(Core :: MFBT, enhancement)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

TL;DR: 'MOZ_NON_PARAM' is theoretically only needed for over-aligned types, which could be determined with `alignof(std::max_align_t)`.

(In reply to Jeff Walden [:Waldo] from bug 1287006 comment #26)
> Use |alignas(T) unsigned char mStorage[sizeof(T)]| instead of AlignedStorage
> inside Maybe
> [...]
> But most important of all, I made Maybe<T> MOZ_NON_PARAM so that Maybe<T>
> can't be a parameter type.  MSVC has a compiler warning (error, because of
> our -Werror) if you try to pass a struct containing an alignas() member by
> value.  This is because alignas (in excess of types' natural alignment?)
> apparently doesn't modify ABI -- and so you're at the mercy of the platform
> ABI whether parameters actually respect alignas.
> 
> This is kind of nuts, IMO.  Excessive alignas *should* affect ABI.  But it
> doesn't.  Given that, I see no way we can ensure alignment of Maybe
> parameters.  So the best we can do is prohibit Maybe being a parameter type.

According to the C++14 specs, 3.11#3, "An extended alignment is represented by an alignment greater than alignof(std::max_align_t). It is implementation-defined whether any extended alignments are supported and the contexts in which they are supported".
So it is in fact correct for an implementation not to accept over-extended types in some situations, and bug 1287006 took the safest option that covers this.

But for non-over extended types, 3.11#2: "A fundamental alignment is represented by an alignment less than or equal to the greatest alignment supported by the implementation in all contexts, which is equal to alignof(std::max_align_t)".

Now, the annoying MSVC warning is C2718 [1]. The wording there says "The align __declspec modifier is not permitted on function parameters", which unfortunately implies that *any* alignment requirement would be ignored. This does seem nuts!

The MSVC docs [2] confirm that "You cannot specify alignment for function parameters. When data that has an alignment attribute is passed by value on the stack, its alignment is controlled by the calling convention.". (Where can I find this calling convention?)
The docs also say "Without __declspec(align(#)), Visual C++ generally aligns data on natural boundaries based on the target processor and the size of the data, up to 4-byte boundaries on 32-bit processors, and 8-byte boundaries on 64-bit processors." So maybe there is still some hope for small-enough data?

I could not reproduce the warning, except on one online 32-bit MSVC compiler (at gcc.godbolt.org), making it difficult to investigate further.
Most unfortunately, `alignof(std::max_align_t)` still gives 8 on that 32-bit build that complained about an 8-byte alignment, so we'd have to manually override it there. :-(


Jeff, you worked on bug 1287006 and discovered C2718, what do you think?
Would it be worth allowing small Maybes in functions again?
(WONTFIX may be a reasonable response here; but I still wanted to scratch this itch, in case there's a useful solution.)


[1] https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-2/compiler-error-c2718
[2] https://docs.microsoft.com/en-us/cpp/cpp/align-cpp

Comment 1

[Jeff Walden [:Waldo]]

The level of grunge your comment is getting in to, to determine that it might sometimes be safe to pass Maybe by value, is more than I care to add to Maybe's interface.  The consistency of "you can't use Maybe as a parameter" and "never use Maybe as a parameter" beats out having Maybe *sometimes* work as parameter, *sometimes* not, or *sometimes* if you're in platform-specific code *and* you've made sure your particular use has jumped through the right hoops that are not at all obvious unless you deeply understand this stuff.

So, yeah -- WONTFIX.