From a8080e8e6f96126705b5d2e6aa8fad590bcaa77e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 20 Dec 2020 11:58:26 -0800 Subject: [PATCH] Allow specifying a limit on number of expansion in operation_context If the user types something like `/**`, prior to this change we would attempt to expand it in the background for both highlighting and autosuggestions. This could thrash your disk and also consume a lot of memory. Add a a field to operation_context_t to allow specifying a limit, and add a "default background" limit of 512 items. --- src/complete.cpp | 2 +- src/complete.h | 7 ++----- src/expand.cpp | 2 +- src/operation_context.cpp | 8 ++++++-- src/operation_context.h | 23 +++++++++++++++-------- src/reader.cpp | 2 +- 6 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/complete.cpp b/src/complete.cpp index 06e682e05..457cd8b90 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -451,7 +451,7 @@ class completer_t { public: completer_t(const operation_context_t &ctx, completion_request_flags_t f) - : ctx(ctx), flags(f) {} + : ctx(ctx), flags(f), completions(ctx.expansion_limit) {} void perform_for_commandline(wcstring cmdline); diff --git a/src/complete.h b/src/complete.h index e8c85e146..04d06a57f 100644 --- a/src/complete.h +++ b/src/complete.h @@ -124,14 +124,11 @@ using completion_list_t = std::vector; /// some conveniences. class completion_receiver_t { public: - /// The default maximum number of items that something may expand to. - static constexpr size_t k_default_expansion_limit = 512 * 1024; - /// Construct as empty, with a limit. - explicit completion_receiver_t(size_t limit = k_default_expansion_limit) : limit_(limit) {} + explicit completion_receiver_t(size_t limit) : limit_(limit) {} /// Acquire an existing list, with a limit. - explicit completion_receiver_t(completion_list_t &&v, size_t limit = k_default_expansion_limit) + explicit completion_receiver_t(completion_list_t &&v, size_t limit) : completions_(std::move(v)), limit_(limit) {} /// Add a completion. diff --git a/src/expand.cpp b/src/expand.cpp index 45b605650..0b1890701 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -1146,7 +1146,7 @@ expand_result_t expander_t::expand_string(wcstring input, completion_receiver_t expand_result_t expand_string(wcstring input, completion_list_t *out_completions, expand_flags_t flags, const operation_context_t &ctx, parse_error_list_t *errors) { - completion_receiver_t recv(std::move(*out_completions)); + completion_receiver_t recv(std::move(*out_completions), ctx.expansion_limit); auto res = expand_string(std::move(input), &recv, flags, ctx, errors); *out_completions = recv.take(); return res; diff --git a/src/operation_context.cpp b/src/operation_context.cpp index 3440fd884..f0f4d301f 100644 --- a/src/operation_context.cpp +++ b/src/operation_context.cpp @@ -8,8 +8,12 @@ bool no_cancel() { return false; } operation_context_t::operation_context_t(std::shared_ptr parser, - const environment_t &vars, cancel_checker_t cancel_checker) - : parser(std::move(parser)), vars(vars), cancel_checker(std::move(cancel_checker)) {} + const environment_t &vars, cancel_checker_t cancel_checker, + size_t expansion_limit) + : parser(std::move(parser)), + vars(vars), + expansion_limit(expansion_limit), + cancel_checker(std::move(cancel_checker)) {} operation_context_t operation_context_t::empty() { static const null_environment_t nullenv{}; diff --git a/src/operation_context.h b/src/operation_context.h index 9e6ae73bc..56b731d99 100644 --- a/src/operation_context.h +++ b/src/operation_context.h @@ -12,14 +12,17 @@ class job_group_t; /// A common helper which always returns false. bool no_cancel(); +/// Default limits for expansion. +enum expansion_limit_t : size_t { + /// The default maximum number of items from expansion. + kExpansionLimitDefault = 512 * 1024, + + /// A smaller limit for background operations like syntax highlighting. + kExpansionLimitBackground = 512, +}; + /// A operation_context_t is a simple property bag which wraps up data needed for highlighting, /// expansion, completion, and more. -/// It contains the following triple: -/// 1. A parser. This is often null. If not null, it may be used to execute fish script. If null, -/// then this is a background operation and fish script must not be executed. -/// 2. A variable set. This is never null. This may differ from the variables in the parser. -/// 3. A cancellation checker. This is a function which you may call to detect that the operation -/// is no longer necessary and should be cancelled. class operation_context_t { public: // The parser, if this is a foreground operation. If this is a background operation, this may be @@ -30,6 +33,9 @@ class operation_context_t { // context itself. const environment_t &vars; + // The limit in the number of expansions which should be produced. + const size_t expansion_limit; + /// The job group of the parental job. /// This is used only when expanding command substitutions. If this is set, any jobs created by /// the command substitions should use this tree. @@ -48,9 +54,10 @@ class operation_context_t { // cancels. static operation_context_t globals(); - /// Construct from the full triple of a parser, vars, and cancel checker. + /// Construct from a full set of properties. operation_context_t(std::shared_ptr parser, const environment_t &vars, - cancel_checker_t cancel_checker); + cancel_checker_t cancel_checker, + size_t expansion_limit = kExpansionLimitDefault); /// Construct from vars alone. explicit operation_context_t(const environment_t &vars) diff --git a/src/reader.cpp b/src/reader.cpp index 2bd5836dd..62f8c3e1f 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -138,7 +138,7 @@ static operation_context_t get_bg_context(const std::shared_ptr & // Cancel if the generation count changed. return generation_count != read_generation_count(); }; - return operation_context_t{nullptr, *env, std::move(cancel_checker)}; + return operation_context_t{nullptr, *env, std::move(cancel_checker), kExpansionLimitBackground}; } /// We try to ensure that syntax highlighting completes appropriately before executing what the user