1447849 - Eliminate the anonymous union from XPTTypeDescriptor

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Andrew McCreight [:mccr8]]

For bug 1438688, I need to statically initialize all of the data types in xpt_struct.h. It turns out that the approach I was using for unions is C99 and not C++. While for some reason Clang and GCC are okay with that, MSVC is not.

One of the unions is a gnarly anonymous union in XPTTypeDescriptor. However, every arm of this union is either 1 or 2 uint8_t, so I think it is reasonable to eliminate the union and replace it with two fields, mData1 and mData2. I've fixed up the places that read this data to use accessor methods with nice asserts about the tag on the variant, so if anything I think that part is nicer.

The code in xpt_struct.cpp that writes to this data is maybe less nice, but I'm deleting it in bug 1438688 so I think that's okay. We also lose some detail in the comments about the relationship between what is stored in memory compared to what is stored on disk, but I think that's okay, too.

Comment 1

[Andrew McCreight [:mccr8]]

Attachment: Bug 1447849 - Eliminate the anonymous union from XPTTypeDescriptor.

For bug 1438688, I need to statically initialize all of the data types
in xpt_struct.h. It turns out that the approach I was using for unions
is C99 and not C++. While for some reason Clang and GCC are okay with
that, MSVC is not.

One of the unions is a gnarly anonymous union in
XPTTypeDescriptor. However, every arm of this union is either 1 or 2
uint8_t, so I think it is reasonable to eliminate the union and
replace it with two fields, mData1 and mData2. I've fixed up the
places that read this data to use accessor methods with nice asserts
about the tag on the variant, so if anything I think that part is
nicer.

The code in xpt_struct.cpp that writes to this data is maybe less
nice, but I'm deleting it in bug 1438688 so I think that's okay. We
also lose some detail in the comments about the relationship between
what is stored in memory compared to what is stored on disk, but I
think that's okay, too.

Review commit: https://reviewboard.mozilla.org/r/230026/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230026/

Comment 2

[Nicholas Nethercote [inactive]]

Comment on attachment 8961250 [details]
Bug 1447849 - Eliminate the anonymous union from XPTTypeDescriptor.

https://reviewboard.mozilla.org/r/230026/#review235736

Fair enough.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d1e7d52a4765
Eliminate the anonymous union from XPTTypeDescriptor. r=njn

Comment 4

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/d1e7d52a4765