diff --git a/src/builtin.cpp b/src/builtin.cpp index 1b935942b..23f905338 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -460,7 +460,7 @@ proc_status_t builtin_run(parser_t &parser, const wcstring_list_t &argv, io_stre // Resolve our status code. // If the builtin itself produced an error, use that error. // Otherwise use any errors from writing to out and writing to err, in that order. - int code = builtin_ret ? *builtin_ret : 0; + int code = builtin_ret.has_value() ? *builtin_ret : 0; if (code == 0) code = out_ret; if (code == 0) code = err_ret; diff --git a/src/builtins/disown.cpp b/src/builtins/disown.cpp index 09176f628..8ae8bc0c8 100644 --- a/src/builtins/disown.cpp +++ b/src/builtins/disown.cpp @@ -28,7 +28,7 @@ static void disown_job(const wchar_t *cmd, io_streams_t &streams, job_t *j) { // Stopped disowned jobs must be manually signaled; explain how to do so. auto pgid = j->get_pgid(); if (j->is_stopped()) { - if (pgid) killpg(*pgid, SIGCONT); + if (pgid.has_value()) killpg(*pgid, SIGCONT); const wchar_t *fmt = _(L"%ls: job %d ('%ls') was stopped and has been signalled to continue.\n"); streams.err.append_format(fmt, cmd, j->job_id(), j->command_wcstr()); diff --git a/src/builtins/jobs.cpp b/src/builtins/jobs.cpp index 3940ba728..e6223ec01 100644 --- a/src/builtins/jobs.cpp +++ b/src/builtins/jobs.cpp @@ -42,8 +42,11 @@ static double cpu_use(const job_t *j) { /// Print information about the specified job. static void builtin_jobs_print(const job_t *j, int mode, int header, io_streams_t &streams) { int pgid = INVALID_PID; - if (auto job_pgid = j->get_pgid()) { - pgid = *job_pgid; + { + auto job_pgid = j->get_pgid(); + if (job_pgid.has_value()) { + pgid = *job_pgid; + } } wcstring out; diff --git a/src/builtins/path.cpp b/src/builtins/path.cpp index 07d8cb92e..42c7f3472 100644 --- a/src/builtins/path.cpp +++ b/src/builtins/path.cpp @@ -671,7 +671,7 @@ static int path_extension(parser_t &parser, io_streams_t &streams, int argc, con while (const wcstring *arg = aiter.nextstr()) { auto pos = find_extension(*arg); - if (!pos) { + if (!pos.has_value()) { // If there is no extension the extension is empty. // This is unambiguous because we include the ".". path_out(streams, opts, L""); @@ -702,7 +702,7 @@ static int path_change_extension(parser_t &parser, io_streams_t &streams, int ar auto pos = find_extension(*arg); wcstring ext; - if (!pos) { + if (!pos.has_value()) { ext = *arg; } else { ext = arg->substr(0, *pos); diff --git a/src/builtins/string.cpp b/src/builtins/string.cpp index 88782a826..9f1dbf57f 100644 --- a/src/builtins/string.cpp +++ b/src/builtins/string.cpp @@ -224,7 +224,7 @@ static size_t width_without_escapes(const wcstring &ins, size_t start_pos = 0) { size_t pos = start_pos; while ((pos = ins.find('\x1B', pos)) != std::string::npos) { auto len = escape_code_length(ins.c_str() + pos); - if (len) { + if (len.has_value()) { auto sub = ins.substr(pos, *len); for (auto c : sub) { auto w = fish_wcwidth_visible(c); @@ -1217,7 +1217,8 @@ static maybe_t interpret_escapes(const wcstring &arg) { const wchar_t *end = cursor + arg.size(); while (cursor < end) { if (*cursor == L'\\') { - if (auto escape_len = read_unquoted_escape(cursor, &result, true, false)) { + auto escape_len = read_unquoted_escape(cursor, &result, true, false); + if (escape_len.has_value()) { cursor += *escape_len; } else { // Invalid escape. @@ -1725,7 +1726,7 @@ static int string_shorten(parser_t &parser, io_streams_t &streams, int argc, con size_t totallen = 0; while (l[pos + totallen] == L'\x1B') { auto len = escape_code_length(l.c_str() + pos + totallen); - if (!len) break; + if (!len.has_value()) break; totallen += *len; } return totallen; diff --git a/src/common.cpp b/src/common.cpp index dd9e691be..50b63edaf 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1370,7 +1370,7 @@ static bool unescape_string_internal(const wchar_t *const input, const size_t in // appending INTERNAL_SEPARATORs, so we have to handle them specially. auto escape_chars = read_unquoted_escape( input + input_position, &result, allow_incomplete, unescape_special); - if (!escape_chars) { + if (!escape_chars.has_value()) { // A none() return indicates an error. errored = true; } else { diff --git a/src/complete.cpp b/src/complete.cpp index f712ae276..2a6714c73 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -1308,7 +1308,7 @@ cleanup_t completer_t::apply_var_assignments(const wcstring_list_t &var_assignme const block_t *block = ctx.parser->push_block(block_t::variable_assignment_block()); for (const wcstring &var_assign : var_assignments) { maybe_t equals_pos = variable_assignment_equals_pos(var_assign); - assert(equals_pos && "All variable assignments should have equals position"); + assert(equals_pos.has_value() && "All variable assignments should have equals position"); const wcstring variable_name = var_assign.substr(0, *equals_pos); const wcstring expression = var_assign.substr(*equals_pos + 1); @@ -1395,7 +1395,7 @@ void completer_t::walk_wrap_chain(const wcstring &cmd, const wcstring &cmdline, size_t wrapped_command_offset_in_wt = wcstring::npos; while (auto tok = tokenizer.next()) { wcstring tok_src = tok->get_source(wt); - if (variable_assignment_equals_pos(tok_src)) { + if (variable_assignment_equals_pos(tok_src).has_value()) { ad->var_assignments->push_back(std::move(tok_src)); } else { wrapped_command_offset_in_wt = tok->offset; @@ -1542,7 +1542,7 @@ void completer_t::perform_for_commandline(wcstring cmdline) { for (const tok_t &tok : tokens) { if (tok.location_in_or_at_end_of_source_range(cursor_pos)) break; wcstring tok_src = tok.get_source(cmdline); - if (!variable_assignment_equals_pos(tok_src)) break; + if (!variable_assignment_equals_pos(tok_src).has_value()) break; var_assignments.push_back(std::move(tok_src)); } tokens.erase(tokens.begin(), tokens.begin() + var_assignments.size()); @@ -1585,7 +1585,7 @@ void completer_t::perform_for_commandline(wcstring cmdline) { if (cmd_tok.location_in_or_at_end_of_source_range(cursor_pos)) { maybe_t equal_sign_pos = variable_assignment_equals_pos(current_token); - if (equal_sign_pos) { + if (equal_sign_pos.has_value()) { complete_param_expand(current_token, true /* do_file */); return; } diff --git a/src/env.cpp b/src/env.cpp index 85ca447fd..282935dbb 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1179,7 +1179,8 @@ mod_result_t env_stack_impl_t::set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t val) { const query_t query(mode); // Handle electric and read-only variables. - if (auto ret = try_set_electric(key, query, val)) { + auto ret = try_set_electric(key, query, val); + if (ret.has_value()) { return mod_result_t{*ret}; } diff --git a/src/exec.cpp b/src/exec.cpp index 59e72af2e..7fae21c3d 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -415,9 +415,12 @@ static launch_result_t fork_child_for_process(const std::shared_ptr &job, if (p->leads_pgrp) { job->group->set_pgid(p->pid); } - if (auto pgid = job->group->get_pgid()) { - if (int err = execute_setpgid(p->pid, *pgid, is_parent)) { - report_setpgid_error(err, is_parent, *pgid, job.get(), p); + { + auto pgid = job->group->get_pgid(); + if (pgid.has_value()) { + if (int err = execute_setpgid(p->pid, *pgid, is_parent)) { + report_setpgid_error(err, is_parent, *pgid, job.get(), p); + } } } @@ -1115,7 +1118,8 @@ bool exec_job(parser_t &parser, const shared_ptr &j, const io_chain_t &bl // If exec_error then a backgrounded job would have been terminated before it was ever assigned // a pgroup, so error out before setting last_pid. if (!j->is_foreground()) { - if (maybe_t last_pid = j->get_last_pid()) { + maybe_t last_pid = j->get_last_pid(); + if (last_pid.has_value()) { parser.vars().set_one(L"last_pid", ENV_GLOBAL, to_string(*last_pid)); } else { parser.vars().set_empty(L"last_pid", ENV_GLOBAL); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 20269d2f8..bb194127a 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2915,7 +2915,7 @@ static bool run_one_test_test(int expected, const wcstring_list_t &lst, bool bra maybe_t result = builtin_test(parser, streams, cargv.get()); if (result != expected) { - std::wstring got = result ? std::to_wstring(result.value()) : L"nothing"; + std::wstring got = result.has_value() ? std::to_wstring(result.value()) : L"nothing"; err(L"expected builtin_test() to return %d, got %s", expected, got.c_str()); } return result == expected; @@ -5895,7 +5895,9 @@ static void run_one_string_test(const wchar_t *const *argv_raw, int expected_rc, args.resize(args.size() - 1); if (rc != expected_rc) { - std::wstring got = rc ? std::to_wstring(rc.value()) : L"nothing"; + // The comparison above would have panicked if rc didn't have a value, so it's safe to + // assume it has one here: + std::wstring got = std::to_wstring(rc.value()); err(L"Test failed on line %lu: [%ls]: expected return code %d but got %s", __LINE__, args.c_str(), expected_rc, got.c_str()); } else if (outs.contents() != expected_out) { @@ -6371,7 +6373,8 @@ static void test_illegal_command_exit_code() { void test_maybe() { say(L"Testing maybe_t"); - do_test(!bool(maybe_t())); + // maybe_t bool conversion is only enabled for non-bool-convertible T types + do_test(!bool(maybe_t())); maybe_t m(3); do_test(m.has_value()); do_test(m.value() == 3); @@ -6389,8 +6392,8 @@ void test_maybe() { do_test(m != maybe_t()); do_test(maybe_t() == none()); do_test(!maybe_t(none()).has_value()); - m = none(); - do_test(!bool(m)); + maybe_t n = none(); + do_test(!bool(n)); maybe_t m2("abc"); do_test(!m2.missing_or_empty()); diff --git a/src/highlight.cpp b/src/highlight.cpp index 2567390b2..d55a17768 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -1026,7 +1026,8 @@ void highlighter_t::visit(const ast::argument_t &arg, bool cmd_is_cd, bool optio void highlighter_t::visit(const ast::variable_assignment_t &varas) { color_as_argument(varas); // Highlight the '=' in variable assignments as an operator. - if (auto where = variable_assignment_equals_pos(varas.source(this->buff))) { + auto where = variable_assignment_equals_pos(varas.source(this->buff)); + if (where.has_value()) { size_t equals_loc = varas.source_range().start + *where; this->color_array.at(equals_loc) = highlight_role_t::operat; auto var_name = varas.source(this->buff).substr(0, *where); @@ -1048,7 +1049,7 @@ void highlighter_t::visit(const ast::decorated_statement_t &stmt) { if (!this->io_ok) { // We cannot check if the command is invalid, so just assume it's valid. is_valid_cmd = true; - } else if (variable_assignment_equals_pos(*cmd)) { + } else if (variable_assignment_equals_pos(*cmd).has_value()) { is_valid_cmd = true; } else { // Check to see if the command is valid. diff --git a/src/history.cpp b/src/history.cpp index 80876d587..41735bfcd 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -582,7 +582,9 @@ void history_impl_t::populate_from_file_contents() { old_item_offsets.clear(); if (file_contents) { size_t cursor = 0; - while (auto offset = file_contents->offset_of_next_item(&cursor, boundary_timestamp)) { + maybe_t offset; + while ((offset = + file_contents->offset_of_next_item(&cursor, boundary_timestamp)).has_value()) { // Remember this item. old_item_offsets.push_back(*offset); } @@ -725,7 +727,8 @@ bool history_impl_t::rewrite_to_temporary_file(int existing_fd, int dst_fd) cons // old file contents). if (auto local_file = history_file_contents_t::create(existing_fd)) { size_t cursor = 0; - while (auto offset = local_file->offset_of_next_item(&cursor, 0)) { + maybe_t offset; + while ((offset = local_file->offset_of_next_item(&cursor, 0)).has_value()) { // Try decoding an old item. history_item_t old_item = local_file->decode_item(*offset); diff --git a/src/maybe.h b/src/maybe.h index ea3938b03..d122677d4 100644 --- a/src/maybe.h +++ b/src/maybe.h @@ -134,8 +134,14 @@ class maybe_t : private maybe_detail::conditionally_copyable_t { // return whether the receiver contains a value. bool has_value() const { return impl_.filled; } - // bool conversion indicates whether the receiver contains a value. - explicit operator bool() const { return impl_.filled; } + // A bool operator as a shortcut to test if the maybe_t has a value. + // Not enabled if the type T is already bool-convertible to prevent accidental misuse, + // otherwise the "typename std::enable_if<....>::type" evaluates to bool, giving us a definition + // of `explicit operator bool() const { ... }` + template explicit operator + typename std::enable_if::value, bool>::type() const { + return impl_.filled; + } // The default constructor constructs a maybe with no value. maybe_t() = default; diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 707c12d3a..dcf839dbf 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1021,7 +1021,8 @@ end_execution_reason_t parse_execution_context_t::determine_redirections( redirection_spec_t spec{oper->fd, oper->mode, std::move(target)}; // Validate this spec. - if (spec.mode == redirection_mode_t::fd && !spec.is_close() && !spec.get_target_as_fd()) { + if (spec.mode == redirection_mode_t::fd && !spec.is_close() && + !spec.get_target_as_fd().has_value()) { const wchar_t *fmt = _(L"Requested redirection to '%ls', which is not a valid file descriptor"); return report_error(STATUS_INVALID_ARGS, redir_node, fmt, spec.target.c_str()); @@ -1095,7 +1096,7 @@ end_execution_reason_t parse_execution_context_t::apply_variable_assignments( for (const ast::variable_assignment_t &variable_assignment : variable_assignment_list) { const wcstring &source = get_source(variable_assignment); auto equals_pos = variable_assignment_equals_pos(source); - assert(equals_pos); + assert(equals_pos.has_value()); const wcstring variable_name = source.substr(0, *equals_pos); const wcstring expression = source.substr(*equals_pos + 1); completion_list_t expression_expanded; diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index a67e8f88f..da64e7424 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -53,7 +53,7 @@ wcstring parse_error_t::describe_with_prefix(const wcstring &src, const wcstring case parse_error_bare_variable_assignment: { wcstring assignment_src = src.substr(this->source_start, this->source_length); maybe_t equals_pos = variable_assignment_equals_pos(assignment_src); - assert(equals_pos); + assert(equals_pos.has_value()); wcstring variable = assignment_src.substr(0, *equals_pos); wcstring value = assignment_src.substr(*equals_pos + 1); append_format(result, ERROR_BAD_COMMAND_ASSIGN_ERR_MSG, variable.c_str(), diff --git a/src/postfork.cpp b/src/postfork.cpp index 151556f79..a96d99cde 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -256,10 +256,10 @@ bool posix_spawner_t::check_fail(int err) { } posix_spawner_t::~posix_spawner_t() { - if (attr_) { + if (attr_.has_value()) { posix_spawnattr_destroy(this->attr()); } - if (actions_) { + if (actions_.has_value()) { posix_spawn_file_actions_destroy(this->actions()); } } @@ -281,10 +281,13 @@ posix_spawner_t::posix_spawner_t(const job_t *j, const dup2_list_t &dup2s) { // desired_pgid tracks the pgroup for the process. If it is none, the pgroup is left unchanged. // If it is zero, create a new pgroup from the pid. If it is >0, join that pgroup. maybe_t desired_pgid = none(); - if (auto pgid = j->group->get_pgid()) { - desired_pgid = *pgid; - } else if (j->processes.front()->leads_pgrp) { - desired_pgid = 0; + { + auto pgid = j->group->get_pgid(); + if (pgid.has_value()) { + desired_pgid = *pgid; + } else if (j->processes.front()->leads_pgrp) { + desired_pgid = 0; + } } // Set the handling for job control signals back to the default. diff --git a/src/proc.cpp b/src/proc.cpp index 4c1b18a50..8be091d50 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -123,7 +123,8 @@ bool job_t::posts_job_exit_events() const { } bool job_t::signal(int signal) { - if (auto pgid = group->get_pgid()) { + auto pgid = group->get_pgid(); + if (pgid.has_value()) { if (killpg(*pgid, signal) == -1) { char buffer[512]; snprintf(buffer, 512, "killpg(%d, %s)", *pgid, strsignal(signal)); @@ -475,7 +476,8 @@ static void generate_job_exit_events(const job_ref_t &j, std::vector *o if (!j->from_event_handler() || !j->is_foreground()) { // job_exit events. if (j->posts_job_exit_events()) { - if (auto last_pid = j->get_last_pid()) { + auto last_pid = j->get_last_pid(); + if (last_pid.has_value()) { out_evts->push_back(event_t::job_exit(*last_pid, j->internal_job_id)); } } @@ -976,7 +978,7 @@ void hup_jobs(const job_list_t &jobs) { pid_t fish_pgrp = getpgrp(); for (const auto &j : jobs) { auto pgid = j->get_pgid(); - if (pgid && *pgid != fish_pgrp && !j->is_completed()) { + if (pgid.has_value() && *pgid != fish_pgrp && !j->is_completed()) { if (j->is_stopped()) { j->signal(SIGCONT); } diff --git a/src/reader.cpp b/src/reader.cpp index 7cdfb5603..8a317d07a 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1103,7 +1103,7 @@ wcstring combine_command_and_autosuggestion(const wcstring &cmdline, /// Update the cursor position. void reader_data_t::update_buff_pos(editable_line_t *el, maybe_t new_pos) { - if (new_pos) { + if (new_pos.has_value()) { el->set_position(*new_pos); } size_t buff_pos = el->position(); diff --git a/src/screen.cpp b/src/screen.cpp index 30e27421b..6e0931f30 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -268,7 +268,7 @@ size_t layout_cache_t::escape_code_length(const wchar_t *code) { if (esc_seq_len) return esc_seq_len; auto found = ::escape_code_length(code); - if (found) { + if (found.has_value()) { this->add_escape_code(wcstring(code, *found)); esc_seq_len = *found; } diff --git a/src/tokenizer.cpp b/src/tokenizer.cpp index b2ceb2879..528d6e68c 100644 --- a/src/tokenizer.cpp +++ b/src/tokenizer.cpp @@ -74,7 +74,7 @@ tok_t tokenizer_t::call_error(tokenizer_error_t error_type, const wchar_t *token result.error = error_type; result.offset = token_start - this->start; // If we are passed a token_length, then use it; otherwise infer it from the buffer. - result.length = token_length ? *token_length : this->token_cursor - token_start; + result.length = token_length.has_value() ? *token_length : this->token_cursor - token_start; result.error_offset_within_token = error_loc - token_start; result.error_length = error_len; return result; @@ -110,7 +110,7 @@ static bool tok_is_string_character(wchar_t c, maybe_t next) { } case L'&': { if (!feature_test(features_t::ampersand_nobg_in_token)) return false; - bool next_is_string = next && tok_is_string_character(*next, none()); + bool next_is_string = next.has_value() && tok_is_string_character(*next, none()); // Unlike in other shells, '&' is not special if followed by a string character. return next_is_string; } @@ -658,7 +658,7 @@ maybe_t tokenizer_t::next() { } bool is_token_delimiter(wchar_t c, maybe_t next) { - return c == L'(' || !tok_is_string_character(c, next); + return c == L'(' || !tok_is_string_character(c, std::move(next)); } wcstring tok_command(const wcstring &str) { @@ -668,7 +668,7 @@ wcstring tok_command(const wcstring &str) { return {}; } wcstring text = t.text_of(*token); - if (variable_assignment_equals_pos(text)) { + if (variable_assignment_equals_pos(text).has_value()) { continue; } return text; @@ -885,7 +885,7 @@ move_word_state_machine_t::move_word_state_machine_t(move_word_style_t syl) void move_word_state_machine_t::reset() { state = 0; } -// Return the location of the equals sign, or npos if the string does +// Return the location of the equals sign, or none if the string does // not look like a variable assignment like FOO=bar. The detection // works similar as in some POSIX shells: only letters and numbers qre // allowed on the left hand side, no quotes or escaping.