From dd61fd6ccfc81cbc0a8c33709300e13920faf71b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 27 Sep 2018 16:13:14 +1000 Subject: [PATCH] Bug 1494515 - Rework the comments about static atoms. r=froydnj nsGkAtoms.h has a big comment explaining how static atom definitions get expanded by macros. This comment was very useful when there were multiple sources of static atoms and their definitions used macros a lot. But bug 1482782 combined all the static atom sources into nsGkAtoms and removed a lot of macro use. So now the comment is now something of a hindrance, duplicating quite a bit of the code (and not entirely accurately). This commit removes the big comment, and moves the still-useful parts inline with the code. This makes things much easier to follow. This commit also reformats some of the remaining macros so they are easier to read. --HG-- extra : rebase_source : 4377be2fa0edc4ea1f592985ded89acbb76fb104 --- xpcom/ds/nsGkAtoms.cpp | 71 +++++++++++++++++--- xpcom/ds/nsGkAtoms.h | 148 ++++++++++------------------------------- 2 files changed, 94 insertions(+), 125 deletions(-) diff --git a/xpcom/ds/nsGkAtoms.cpp b/xpcom/ds/nsGkAtoms.cpp index b5c399ddf4f4..a21b140e1263 100644 --- a/xpcom/ds/nsGkAtoms.cpp +++ b/xpcom/ds/nsGkAtoms.cpp @@ -13,8 +13,17 @@ NS_RegisterStaticAtoms(const nsStaticAtom* aAtoms, size_t aAtomsLen); namespace mozilla { namespace detail { +// Because this is `constexpr` it ends up in read-only memory where it can be +// shared between processes. extern constexpr GkAtoms gGkAtoms = { // The initialization of each atom's string. + // + // Expansion of the example GK_ATOM entries in nsGkAtoms.h: + // + // u"a", + // u"bb", + // u"ccc", + // #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ u"" value_, #include "nsGkAtomList.h" @@ -25,13 +34,30 @@ extern constexpr GkAtoms gGkAtoms = { // Note that |value_| is an 8-bit string, and so |sizeof(value_)| is equal // to the number of chars (including the terminating '\0'). The |u""| prefix // converts |value_| to a 16-bit string. - #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ - nsStaticAtom(u"" value_, \ - sizeof(value_) - 1, \ - hash_, \ - offsetof(GkAtoms, \ - mAtoms[static_cast(GkAtoms::Atoms::name_)]) - \ - offsetof(GkAtoms, name_##_string)), + // + // Expansion of the example GK_ATOM entries in nsGkAtoms.h: + // + // nsStaticAtom( + // u"a", 1, 0x01234567, + // offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::a)]) - + // offsetof(GkAtoms, a_string)), + // + // nsStaticAtom( + // u"bb", 2, 0x12345678, + // offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::bb)]) - + // offsetof(GkAtoms, bb_string)), + // + // nsStaticAtom( + // u"ccc", 3, 0x23456789, + // offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::ccc)]) - + // offsetof(GkAtoms, ccc_string)), + // + #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ + nsStaticAtom( \ + u"" value_, sizeof(value_) - 1, hash_, \ + offsetof(GkAtoms, \ + mAtoms[static_cast(GkAtoms::Atoms::name_)]) - \ + offsetof(GkAtoms, name_##_string)), #include "nsGkAtomList.h" #undef GK_ATOM } @@ -43,10 +69,33 @@ extern constexpr GkAtoms gGkAtoms = { const nsStaticAtom* const nsGkAtoms::sAtoms = mozilla::detail::gGkAtoms.mAtoms; // Definition of the pointer to the static atom. -#define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ - type_* nsGkAtoms::name_ = const_cast(static_cast( \ - &mozilla::detail::gGkAtoms.mAtoms[ \ - static_cast(mozilla::detail::GkAtoms::Atoms::name_)])); +// +// Expansion of the example GK_ATOM entries in nsGkAtoms.h: +// +// nsStaticAtom* nsGkAtoms::a = +// const_cast( +// static_cast( +// &mozilla::detail::gGkAtoms.mAtoms[ +// static_cast(mozilla::detail::GkAtoms::Atoms::a)])); +// +// nsICSSPseudoElement* nsGkAtoms::bb = +// const_cast( +// static_cast( +// &mozilla::detail::gGkAtoms.mAtoms[ +// static_cast(mozilla::detail::GkAtoms::Atoms::bb)])); +// +// nsICSSAnonBoxPseudo* nsGkAtoms::ccc = +// const_cast( +// static_cast( +// &mozilla::detail::gGkAtoms.mAtoms[ +// static_cast(mozilla::detail::GkAtoms::Atoms::ccc)])); +// +#define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ + type_* nsGkAtoms::name_ = \ + const_cast( \ + static_cast( \ + &mozilla::detail::gGkAtoms.mAtoms[ \ + static_cast(mozilla::detail::GkAtoms::Atoms::name_)])); #include "nsGkAtomList.h" #undef GK_ATOM diff --git a/xpcom/ds/nsGkAtoms.h b/xpcom/ds/nsGkAtoms.h index 69d30b70ba30..2bd8c780faed 100644 --- a/xpcom/ds/nsGkAtoms.h +++ b/xpcom/ds/nsGkAtoms.h @@ -43,119 +43,15 @@ // // nsGkAtoms below defines static atoms in a way that satisfies these // constraints. It uses nsGkAtomList.h, which defines the names and values of -// the atoms. +// the atoms. nsGkAtomList.h is generated by StaticAtoms.py and has entries +// that look like this: // -// nsGkAtomList.h is generated by StaticAtoms.py and has entries that look -// like this: +// GK_ATOM(a, "a", 0x01234567, nsStaticAtom, Atom) +// GK_ATOM(bb, "bb", 0x12345678, nsICSSPseudoElement, PseudoElementAtom) +// GK_ATOM(ccc, "ccc", 0x23456789, nsICSSAnonBoxPseudo, InheritingAnonBoxAtom) // -// GK_ATOM(one, "one", 0x01234567, nsStaticAtom, Atom) -// GK_ATOM(two, "two", 0x12345678, nsICSSPseudoElement, PseudoElementAtom) -// GK_ATOM(three, "three", 0x23456789, nsICSSAnonBoxPseudo, InheritingAnonBoxAtom) -// -// After macro expansion, the atom definitions look like the following: -// -// ====> nsGkAtoms.h <==== -// -// namespace mozilla { -// namespace detail { -// -// struct GkAtoms -// { -// // The declaration of each atom's string. -// const char16_t one_string[sizeof("one")]; -// const char16_t two_string[sizeof("two")]; -// const char16_t three_string[sizeof("three")]; -// -// // The enum value for each atom. -// enum class Atoms { -// one_, -// two_, -// three_, -// AtomsCount -// }; -// -// const nsStaticAtom mAtoms[static_cast(Atoms::AtomsCount)]; -// }; -// -// } // namespace detail -// } // namespace mozilla -// -// // This class holds the pointers to the individual atoms. -// class nsGkAtoms -// { -// private: -// // This is a useful handle to the array of atoms, used below and also -// // possibly by Rust code. -// static const nsStaticAtom* const sAtoms; -// -// // The number of atoms, used below. -// static constexpr size_t sAtomsLen = -// static_cast(detail::MyAtoms::Atoms::AtomsCount); -// -// public: -// // These types are not `nsStaticAtom* const`, etc. -- even though these -// // atoms are immutable -- because they are often passed to functions with -// // `nsAtom*` parameters, i.e. that can be passed both dynamic and -// // static. -// static nsStaticAtom* one; -// static nsICSSPseudoElement* two; -// static nsICSSAnonBoxPseudo* three; -// }; -// -// ====> nsGkAtoms.cpp <==== -// -// namespace mozilla { -// namespace detail { -// -// // Need to suppress some MSVC warning weirdness with WrappingMultiply(). -// MOZ_PUSH_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING -// // Because this is `constexpr` it ends up in read-only memory where it can -// // be shared between processes. -// static constexpr GkAtoms gGkAtoms = { -// // The initialization of each atom's string. -// u"one", -// u"two", -// u"three", -// { -// // The initialization of the atoms themselves. -// nsStaticAtom( -// u"one", 3, -// 0x01234567, -// offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::one)]) - -// offsetof(GkAtoms, one_string)), -// nsStaticAtom( -// u"two", 3, -// 0x12345678, -// offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::two)]) - -// offsetof(GkAtoms, two_string)), -// nsStaticAtom( -// u"three", 3, -// 0x23456789, -// offsetof(GkAtoms, mAtoms[static_cast(GkAtoms::Atoms::three)]) - -// offsetof(GkAtoms, three_string)), -// } -// }; -// MOZ_POP_DISABLE_INTEGRAL_CONSTANT_OVERFLOW_WARNING -// -// } // namespace detail -// } // namespace mozilla -// -// const nsStaticAtom* const nsGkAtoms::sAtoms = -// mozilla::detail::gGkAtoms.mAtoms; -// -// // Definition of the pointer to the static atom. -// nsStaticAtom* nsGkAtoms::one = -// const_cast(static_cast( -// &detail::gGkAtoms.mAtoms[ -// static_cast(detail::GkAtoms::Atoms::one)]); -// nsICSSPseudoElement* nsGkAtoms::two = -// const_cast(static_cast( -// &detail::gGkAtoms.mAtoms[ -// static_cast(detail::GkAtoms::Atoms::two)]); -// nsICSSAnonBoxPseudo* nsGkAtoms::three = -// const_cast(static_cast( -// &detail::gGkAtoms.mAtoms[ -// static_cast(detail::GkAtoms::Atoms::three)]); +// Comments throughout this file and nsGkAtoms.cpp show how these entries get +// expanded by macros. // Trivial subclasses of nsStaticAtom so that function signatures can require // an atom from a specific atom list. @@ -176,15 +72,22 @@ DEFINE_STATIC_ATOM_SUBCLASS(nsICSSPseudoElement) namespace mozilla { namespace detail { -// This `detail` class contains the atom strings and the atom objects. -// Because they are together in a class, the mStringOffset field of the -// atoms will be small and can be initialized at compile time. +// This `detail` class contains the atom strings and the atom objects. Because +// they are together in a class, the `mStringOffset` field of the atoms will be +// small and can be initialized at compile time. // // A `detail` namespace is used because the things within it aren't directly // referenced by external users of these static atoms. struct GkAtoms { // The declaration of each atom's string. + // + // Expansion of the example GK_ATOM entries from above: + // + // const char16_t a_string[sizeof("a")]; + // const char16_t bb_string[sizeof("bb")]; + // const char16_t ccc_string[sizeof("ccc")]; + // #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ const char16_t name_##_string[sizeof(value_)]; #include "nsGkAtomList.h" @@ -192,6 +95,12 @@ struct GkAtoms // The enum value for each atom. enum class Atoms { + // Expansion of the example GK_ATOM entries above: + // + // a, + // bb, + // ccc, + // #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \ name_, #include "nsGkAtomList.h" @@ -205,10 +114,15 @@ struct GkAtoms } // namespace detail } // namespace mozilla +// This class holds the pointers to the individual atoms. class nsGkAtoms { private: + // This is a useful handle to the array of atoms, used below and also + // possibly by Rust code. static const nsStaticAtom* const sAtoms; + + // The number of atoms, used below. static constexpr size_t sAtomsLen = static_cast(mozilla::detail::GkAtoms::Atoms::AtomsCount); @@ -223,6 +137,12 @@ public: // The declaration of the pointer to each static atom. // + // Expansion of the example GK_ATOM entries above: + // + // static nsStaticAtom* a; + // static nsICSSPseudoElement* bb; + // static nsICSSAnonBoxPseudo* ccc; + // // XXX: Eventually this should be combined with its definition and the // pointer should be made `constexpr`. See bug 1449787. #define GK_ATOM(name_, value_, hash_, type_, atom_type_) \