From afcd20546c5016caf421440499fa467c0e782a23 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sun, 9 Apr 2023 13:55:28 -0400 Subject: [PATCH] Type checking when building registry trees, except... ... GroupItemBase::push_back is still a hole in the defenses --- libraries/lib-registries/Registry.h | 12 +++-- libraries/lib-snapping/SnapUtils.cpp | 4 +- libraries/lib-snapping/SnapUtils.h | 4 +- src/commands/CommandManager.h | 5 +- src/menus/MenuHelper.cpp | 70 ++++++++++++++++------------ src/widgets/PopupMenuTable.h | 2 +- 6 files changed, 58 insertions(+), 39 deletions(-) diff --git a/libraries/lib-registries/Registry.h b/libraries/lib-registries/Registry.h index b652fe81a177..8d04bec51933 100644 --- a/libraries/lib-registries/Registry.h +++ b/libraries/lib-registries/Registry.h @@ -80,9 +80,9 @@ namespace detail { OrderingHint orderingHint; }; -} using BaseItemPtr = std::unique_ptr; +} class Visitor; @@ -236,7 +236,7 @@ namespace detail { private: friend REGISTRIES_API void detail::RegisterItem(GroupItemBase ®istry, - const Placement &placement, BaseItemPtr pItem); + const Placement &placement, detail::BaseItemPtr pItem); }; // Forward declarations necessary before customizing Composite::Traits @@ -252,11 +252,12 @@ template struct Composite::Traits< static constexpr auto ItemBuilder = Registry::detail::Builder{}; //! Prohibit pushing-back of type-unchecked BaseItemPtr directly - template static constexpr auto enables_item_type_v = true; + template static constexpr auto enables_item_type_v = + !std::is_same_v; }; namespace Registry { - //! Extends GroupItemBase with a variadic constructor + //! Extends GroupItemBase with a variadic constructor that checks types /*! @tparam RegistryTraits defines associated types */ @@ -313,6 +314,7 @@ namespace detail { template auto operator () (std::unique_ptr ptr) const { + static_assert(AcceptableType_v); return move(ptr); } //! This overload allows a lambda or function pointer in the variadic @@ -322,11 +324,13 @@ namespace detail { typename std::invoke_result_t::element_type > auto operator () (const Factory &factory) const { + static_assert(AcceptableType_v); return std::make_unique>(factory); } //! This overload lets you supply a shared pointer to an item, directly template auto operator () (const std::shared_ptr &ptr) const { + static_assert(AcceptableType_v); return std::make_unique>(ptr); } }; diff --git a/libraries/lib-snapping/SnapUtils.cpp b/libraries/lib-snapping/SnapUtils.cpp index 999d1abc8d0e..adf412840261 100644 --- a/libraries/lib-snapping/SnapUtils.cpp +++ b/libraries/lib-snapping/SnapUtils.cpp @@ -336,7 +336,7 @@ class ProjectDependentMultiplierSnapItem final : public SnapRegistryItem } -Registry::BaseItemPtr TimeInvariantSnapFunction( +std::unique_ptr TimeInvariantSnapFunction( const Identifier& functionId, const TranslatableString& label, MultiplierFunctor functor) { @@ -344,7 +344,7 @@ Registry::BaseItemPtr TimeInvariantSnapFunction( functionId, label, std::move(functor)); } -Registry::BaseItemPtr TimeInvariantSnapFunction( +std::unique_ptr TimeInvariantSnapFunction( const Identifier& functionId, const TranslatableString& label, double multiplier) { diff --git a/libraries/lib-snapping/SnapUtils.h b/libraries/lib-snapping/SnapUtils.h index dc4a9e040ffb..593210695ed4 100644 --- a/libraries/lib-snapping/SnapUtils.h +++ b/libraries/lib-snapping/SnapUtils.h @@ -91,13 +91,13 @@ struct SNAPPING_API SnapRegistryItem : public Registry::SingleItem virtual SnapResult SingleStep(const AudacityProject& project, double time, bool upwards) const = 0; }; -SNAPPING_API Registry::BaseItemPtr TimeInvariantSnapFunction( +std::unique_ptr TimeInvariantSnapFunction( const Identifier& functionId, const TranslatableString& label, double multiplier); using MultiplierFunctor = std::function; -SNAPPING_API Registry::BaseItemPtr TimeInvariantSnapFunction( +std::unique_ptr TimeInvariantSnapFunction( const Identifier& functionId, const TranslatableString& label, MultiplierFunctor functor); diff --git a/src/commands/CommandManager.h b/src/commands/CommandManager.h index 17f623d2b28e..592ad2f72c2b 100644 --- a/src/commands/CommandManager.h +++ b/src/commands/CommandManager.h @@ -426,14 +426,17 @@ namespace MenuTable { using namespace Registry; struct CommandItem; + struct CommandGroupItem; struct ConditionalGroupItem; struct MenuItem; struct MenuItems; + struct SpecialItem; struct MenuPart; struct Traits : Registry::DefaultTraits { using ComputedItemContextType = AudacityProject; - using LeafTypes = List; + using LeafTypes = List< + CommandItem, CommandGroupItem, SpecialItem>; using NodeTypes = List< ConditionalGroupItem, MenuItem, MenuItems, MenuPart>; }; diff --git a/src/menus/MenuHelper.cpp b/src/menus/MenuHelper.cpp index 2b47f77bf86e..02ed2138624c 100644 --- a/src/menus/MenuHelper.cpp +++ b/src/menus/MenuHelper.cpp @@ -254,16 +254,6 @@ void AddSortedEffectMenuItems( } } -auto MenuOrItems(const TranslatableString &label) - -> std::unique_ptr -{ - using namespace MenuTable; - if (label.empty()) - return Items(""); - else - return Menu("", label); -} - auto MakeAddGroupItems( const EffectsMenuGroups& list, CommandFlag batchflags, @@ -301,11 +291,21 @@ auto MakeAddGroupItems( if (!groupNames.empty()) { - auto temp = MenuOrItems(p.first); - AddEffectMenuItemGroup(*temp, - groupNames, groupPlugs, groupFlags, - onMenuCommand); - items.push_back(move(temp)); + using namespace MenuTable; + if (p.first.empty()) { + auto temp = Items(""); + AddEffectMenuItemGroup(*temp, + groupNames, groupPlugs, groupFlags, + onMenuCommand); + items.push_back(move(temp)); + } + else { + auto temp = Menu("", p.first); + AddEffectMenuItemGroup(*temp, + groupNames, groupPlugs, groupFlags, + onMenuCommand); + items.push_back(move(temp)); + } } } }; @@ -332,13 +332,22 @@ void AddGroupedEffectMenuItems( auto doAddGroup = [&] { + using namespace MenuTable; if(names.empty()) return; const auto inSubmenu = !path.empty() && (names.size() > 1); - auto items = MenuOrItems(inSubmenu ? path.back() : TranslatableString{}); - AddEffectMenuItemGroup(*items, names, group, flags, onMenuCommand); - parentTable->push_back(move(items)); + const auto label = inSubmenu ? path.back() : TranslatableString{}; + if (label.empty()) { + auto items = Items(""); + AddEffectMenuItemGroup(*items, names, group, flags, onMenuCommand); + parentTable->push_back(move(items)); + } + else { + auto items = Menu("", label); + AddEffectMenuItemGroup(*items, names, group, flags, onMenuCommand); + parentTable->push_back(move(items)); + } names.clear(); group.clear(); @@ -753,16 +762,19 @@ void MenuHelper::PopulateEffectsMenu( if(section.compare != nullptr) std::sort(section.plugins.begin(), section.plugins.end(), section.compare); - std::unique_ptr group; - if (menuItems.empty()) - group = MenuTable::Items(""); - else - group = MenuTable::Section(""); - section.add(*group, section.plugins); - - if (group->empty()) - continue; - - menuItems.push_back(move(group)); + if (menuItems.empty()) { + auto group = MenuTable::Items(""); + section.add(*group, section.plugins); + if (group->empty()) + continue; + menuItems.push_back(move(group)); + } + else { + auto group = MenuTable::Section(""); + section.add(*group, section.plugins); + if (group->empty()) + continue; + menuItems.push_back(move(group)); + } } } diff --git a/src/widgets/PopupMenuTable.h b/src/widgets/PopupMenuTable.h index 4c1a22a1b6de..01ed25621678 100644 --- a/src/widgets/PopupMenuTable.h +++ b/src/widgets/PopupMenuTable.h @@ -227,7 +227,7 @@ class AUDACITY_DLL_API PopupMenuTable : public PopupMenuHandler void BeginSection( const Identifier &name ); void EndSection(); - std::shared_ptr mTop; + std::shared_ptr mTop; std::vector mStack; Identifier mId; TranslatableString mCaption;