[SVG -> OTF Converter] Make Placeholder a move-only type
Created attachment 243631 [details] Patch
I'm not sure if there's a better way to do this other than a movedOutOf boolean.
Created attachment 243636 [details] Patch
Comment on attachment 243636 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243636&action=review > Source/WebCore/svg/SVGToOTFFontConversion.cpp:86 > + , m_movedOutOf(false) This initialization should be !ASSERT_DISABLED only. But it should be omitted entirely (see below). > Source/WebCore/svg/SVGToOTFFontConversion.cpp:103 > +#if !ASSERT_DISABLED > + , m_written(other.m_written) > +#endif Doesn’t seem right to me. We should ASSERT(other.m_written) and just initialize m_written to true. > Source/WebCore/svg/SVGToOTFFontConversion.cpp:106 > + other.m_movedOutOf = true; Should only compile this when !ASSERT_DISABLED. > Source/WebCore/svg/SVGToOTFFontConversion.cpp:123 > - ASSERT(m_written); > + ASSERT(m_movedOutOf || m_written); Change doesn’t make sense to me. No reason to check m_movedOutOf here that I can see. > Source/WebCore/svg/SVGToOTFFontConversion.cpp:130 > + unsigned m_movedOutOf : 1; This should be !ASSERT_DISABLED only. This should be bool, not a bit field, because it’s !ASSERT_DISABLED only and size not all that important. This should be initialized here to cut down on initialization in constructors: bool m_movedOutOf { false }; > Source/WebCore/svg/SVGToOTFFontConversion.cpp:132 > + unsigned m_written : 1; This should be bool, not a bit field, because it’s !ASSERT_DISABLED only and size not all that important. This should be initialized here to cut down on initialization in constructors: bool m_written { false };
Created attachment 243678 [details] Patch
Comment on attachment 243636 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243636&action=review >> Source/WebCore/svg/SVGToOTFFontConversion.cpp:86 >> + , m_movedOutOf(false) > > This initialization should be !ASSERT_DISABLED only. But it should be omitted entirely (see below). I've removed this variable, and renamed m_written to m_active, because a Placeholder which has been moved from but hasn't written anything yet should be marked as inactive. Populate() will then set m_active to false, and the destructor will ASSERT(!m_active);
Comment on attachment 243678 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243678&action=review > Source/WebCore/svg/SVGToOTFFontConversion.cpp:90 > + Placeholder(const Placeholder&) = delete; Implementing a move constructor is enough to not make it have a copy constructor, so this isn't needed.
Committed r177688: <http://trac.webkit.org/changeset/177688>