Repository: microsoft/terminal
File: src/cascadia/TerminalSettingsModel/ActionMap.cpp (C++)

Code before:

using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::Control;

namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
    static InternalActionID Hash(const Model::ActionAndArgs& actionAndArgs)
    {
        size_t hashedAction{ HashUtils::HashProperty(actionAndArgs.Action()) };

        size_t hashedArgs{};
        if (const auto& args{ actionAndArgs.Args() })
        {
            // Args are defined, so hash them
            hashedArgs = gsl::narrow_cast<size_t>(args.Hash());
        }
        else
        {
            // Args are not defined.
            // Check if the ShortcutAction supports args.
            switch (actionAndArgs.Action())
            {
#define ON_ALL_ACTIONS_WITH_ARGS(action)                                    \
    case ShortcutAction::action:                                            \
        /* If it does, hash the default values for the args.*/              \
        hashedArgs = gsl::narrow_cast<size_t>(make<action##Args>().Hash()); \
        break;
                ALL_SHORTCUT_ACTIONS_WITH_ARGS
#undef ON_ALL_ACTIONS_WITH_ARGS
            default:
            {
                // Otherwise, hash nullptr.
                std::hash<IActionArgs> argsHash;
                hashedArgs = argsHash(nullptr);
            }
            }
        }
        return hashedAction ^ hashedArgs;
    }

    ActionMap::ActionMap() :
        _NestedCommands{ single_threaded_map<hstring, Model::Command>() }
    {
    }

    // Method Description:
    // - Retrieves the Command in the current layer, if it's valid
    // - We internally store invalid commands as full commands.
    //    This helper function returns nullptr when we get an invalid
    //    command. This allows us to simply check for null when we
    //    want a valid command.

Review comment (performance):
I low-key hate that this requires _constructing an entire object filled with default values_ to calculate a hash; it could be wildly expensive. Can you profile to make sure we're not making a bunch of objects just to destroy them moments later? Both CPU and memory-wise.

Code after:

using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace winrt::Microsoft::Terminal::Control;

namespace winrt::Microsoft::Terminal::Settings::Model::implementation
{
    static InternalActionID Hash(const Model::ActionAndArgs& actionAndArgs)
    {
        size_t hashedAction{ HashUtils::HashProperty(actionAndArgs.Action()) };

        size_t hashedArgs{};
        if (const auto& args{ actionAndArgs.Args() })
        {
            // Args are defined, so hash them
            hashedArgs = gsl::narrow_cast<size_t>(args.Hash());
        }
        else
        {
            // Args are not defined.
            // Check if the ShortcutAction supports args.
            switch (actionAndArgs.Action())
            {
#define ON_ALL_ACTIONS_WITH_ARGS(action)                        \
    case ShortcutAction::action:                                \
        /* If it does, hash the default values for the args.*/  \
        hashedArgs = EmptyHash<implementation::action##Args>(); \
        break;
                ALL_SHORTCUT_ACTIONS_WITH_ARGS
#undef ON_ALL_ACTIONS_WITH_ARGS
            default:
            {
                // Otherwise, hash nullptr.
                std::hash<IActionArgs> argsHash;
                hashedArgs = argsHash(nullptr);
            }
            }
        }
        return hashedAction ^ hashedArgs;
    }

    ActionMap::ActionMap() :
        _NestedCommands{ single_threaded_map<hstring, Model::Command>() },
        _IterableCommands{ single_threaded_vector<Model::Command>() }
    {
    }

    // Method Description:
    // - Retrieves the Command in the current layer, if it's valid
    // - We internally store invalid commands as full commands.
    //    This helper function returns nullptr when we get an invalid
    //    command. This allows us to simply check for null when we