From 1c1aa2c0d0fea92ed71d5589f69cfc8983ab11b3 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Fri, 30 Jun 2023 01:24:52 +0330 Subject: [PATCH] Shell: Handle (most) errors in the parsers This turns all errors into either "OOM" or a proper shell error (if propagation is impossible or meaningless). Fixes `echo -en '\xfe\x4a' | $SHELL` crashing. --- Userland/Shell/Parser.cpp | 225 ++++++++++++++++++++------------- Userland/Shell/PosixParser.cpp | 99 +++++++++------ 2 files changed, 200 insertions(+), 124 deletions(-) diff --git a/Userland/Shell/Parser.cpp b/Userland/Shell/Parser.cpp index c2c9cd5f78..44d92a4c73 100644 --- a/Userland/Shell/Parser.cpp +++ b/Userland/Shell/Parser.cpp @@ -15,6 +15,48 @@ #include #include +#define TRY_OR_THROW_PARSE_ERROR(expr) ({ \ + /* Ignore -Wshadow to allow nesting the macro. */ \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto&& _value_or_error = expr;) \ + if (_value_or_error.is_error()) { \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto _error = _value_or_error.release_error();) \ + if (_error.is_errno() && _error.code() == ENOMEM) \ + return create("OOM"_short_string); \ + return create(MUST(String::formatted("Error: {}", _error))); \ + } \ + _value_or_error.release_value(); \ +}) + +#define TRY_OR_RESOLVE_TO_ERROR_STRING(expr) ({ \ + /* Ignore -Wshadow to allow nesting the macro. */ \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto&& _value_or_error = expr; \ + String _string_value;) \ + if (_value_or_error.is_error()) { \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto _error = _value_or_error.release_error();) \ + if (_error.is_errno() && _error.code() == ENOMEM) \ + _string_value = "OOM"_short_string; \ + else \ + _string_value = MUST(String::formatted("Error: {}", _error)); \ + } \ + _value_or_error.is_error() ? _string_value : _value_or_error.release_value(); \ +}) + +#define TRY_OR(expr, catch_expr) ({ \ + /* Ignore -Wshadow to allow nesting the macro. */ \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto&& _value_or_error = expr;) \ + if (_value_or_error.is_error()) { \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto _error = _value_or_error.release_error();) \ + catch_expr; \ + } \ + _value_or_error.release_value(); \ +}) + namespace Shell { Parser::SavedOffset Parser::save_offset() const @@ -132,7 +174,7 @@ RefPtr Parser::parse() auto error_start = push_start(); while (!at_end()) consume(); - auto syntax_error_node = create("Unexpected tokens past the end"_string.release_value_but_fixme_should_propagate_errors()); + auto syntax_error_node = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Unexpected tokens past the end"_string)); if (!toplevel) toplevel = move(syntax_error_node); else if (!toplevel->is_syntax_error()) @@ -217,7 +259,7 @@ Parser::SequenceParseResult Parser::parse_sequence() error_builder.appendff(", {} (at {}:{})", entry.end, entry.node->position().start_line.line_column, entry.node->position().start_line.line_number); first = false; } - left.append(create(error_builder.to_string().release_value_but_fixme_should_propagate_errors(), true)); + left.append(create(TRY_OR_RESOLVE_TO_ERROR_STRING(error_builder.to_string()), true)); // Just read the rest of the newlines goto discard_terminators; } @@ -314,7 +356,7 @@ RefPtr Parser::parse_variable_decls() return nullptr; } - auto name_expr = create(String::from_utf8(var_name).release_value_but_fixme_should_propagate_errors()); + auto name_expr = create(TRY_OR_THROW_PARSE_ERROR(String::from_utf8(var_name))); auto start = push_start(); auto expression = parse_expression(); @@ -326,7 +368,7 @@ RefPtr Parser::parse_variable_decls() if (!command) restore_to(*start); else if (!expect(')')) - command->set_is_syntax_error(*create("Expected a terminating close paren"_string.release_value_but_fixme_should_propagate_errors(), true)); + command->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a terminating close paren"_string), true)); expression = command; } } @@ -391,7 +433,7 @@ RefPtr Parser::parse_function_decl() // FIXME: Should this be a syntax error, or just return? return restore(); } - arguments.append({ String::from_utf8(arg_name).release_value_but_fixme_should_propagate_errors(), { name_offset, m_offset, start_line, line() } }); + arguments.append({ TRY_OR_THROW_PARSE_ERROR(String::from_utf8(arg_name)), { name_offset, m_offset, start_line, line() } }); } consume_while(is_any_of("\n\t "sv)); @@ -400,12 +442,12 @@ RefPtr Parser::parse_function_decl() RefPtr syntax_error; { auto obrace_error_start = push_start(); - syntax_error = create("Expected an open brace '{' to start a function body"_string.release_value_but_fixme_should_propagate_errors(), true); + syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start a function body"_string), true); } if (!expect('{')) { return create( AST::NameWithPosition { - String::from_utf8(function_name).release_value_but_fixme_should_propagate_errors(), + TRY_OR_THROW_PARSE_ERROR(String::from_utf8(function_name)), { pos_before_name.offset, pos_after_name.offset, pos_before_name.line, pos_after_name.line } }, move(arguments), move(syntax_error)); @@ -419,7 +461,7 @@ RefPtr Parser::parse_function_decl() RefPtr syntax_error; { auto cbrace_error_start = push_start(); - syntax_error = create("Expected a close brace '}' to end a function body"_string.release_value_but_fixme_should_propagate_errors(), true); + syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a function body"_string), true); } if (!expect('}')) { if (body) @@ -429,7 +471,7 @@ RefPtr Parser::parse_function_decl() return create( AST::NameWithPosition { - String::from_utf8(function_name).release_value_but_fixme_should_propagate_errors(), + TRY_OR_THROW_PARSE_ERROR(String::from_utf8(function_name)), { pos_before_name.offset, pos_after_name.offset, pos_before_name.line, pos_after_name.line } }, move(arguments), move(body)); @@ -438,7 +480,7 @@ RefPtr Parser::parse_function_decl() return create( AST::NameWithPosition { - String::from_utf8(function_name).release_value_but_fixme_should_propagate_errors(), + TRY_OR_THROW_PARSE_ERROR(String::from_utf8(function_name)), { pos_before_name.offset, pos_after_name.offset, pos_before_name.line, pos_after_name.line } }, move(arguments), move(body)); @@ -460,7 +502,7 @@ RefPtr Parser::parse_or_logical_sequence() auto right_and_sequence = parse_and_logical_sequence(); if (!right_and_sequence) - right_and_sequence = create("Expected an expression after '||'"_string.release_value_but_fixme_should_propagate_errors(), true); + right_and_sequence = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an expression after '||'"_string), true); return create( and_sequence.release_nonnull(), @@ -484,7 +526,7 @@ RefPtr Parser::parse_and_logical_sequence() auto right_and_sequence = parse_and_logical_sequence(); if (!right_and_sequence) - right_and_sequence = create("Expected an expression after '&&'"_string.release_value_but_fixme_should_propagate_errors(), true); + right_and_sequence = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an expression after '&&'"_string), true); return create( pipe_sequence.release_nonnull(), @@ -642,7 +684,7 @@ RefPtr Parser::parse_for_loop() auto offset_after_variable = current_position(); index_variable_name = AST::NameWithPosition { - String::from_utf8(variable).release_value_but_fixme_should_propagate_errors(), + TRY_OR_THROW_PARSE_ERROR(String::from_utf8(variable)), { offset_before_variable.offset, offset_after_variable.offset, offset_before_variable.line, offset_after_variable.line }, }; @@ -660,13 +702,13 @@ RefPtr Parser::parse_for_loop() auto variable_name_end_offset = current_position(); if (!name.is_empty()) { variable_name = AST::NameWithPosition { - String::from_utf8(name).release_value_but_fixme_should_propagate_errors(), + TRY_OR_THROW_PARSE_ERROR(String::from_utf8(name)), { variable_name_start_offset.offset, variable_name_end_offset.offset, variable_name_start_offset.line, variable_name_end_offset.line } }; consume_while(is_whitespace); auto in_error_start = push_start(); if (!expect("in"sv)) { - auto syntax_error = create("Expected 'in' after a variable name in a 'for' loop"_string.release_value_but_fixme_should_propagate_errors(), true); + auto syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected 'in' after a variable name in a 'for' loop"_string), true); return create(move(variable_name), move(index_variable_name), move(syntax_error), nullptr); // ForLoop Var Iterated Block } in_start_position = AST::Position { in_error_start->offset, m_offset, in_error_start->line, line() }; @@ -678,14 +720,14 @@ RefPtr Parser::parse_for_loop() auto iter_error_start = push_start(); iterated_expression = parse_expression(); if (!iterated_expression) - iterated_expression = create("Expected an expression in 'for' loop"_string.release_value_but_fixme_should_propagate_errors(), true); + iterated_expression = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an expression in 'for' loop"_string), true); } consume_while(is_any_of(" \t\n"sv)); { auto obrace_error_start = push_start(); if (!expect('{')) { - auto syntax_error = create("Expected an open brace '{' to start a 'for' loop body"_string.release_value_but_fixme_should_propagate_errors(), true); + auto syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start a 'for' loop body"_string), true); return create(move(variable_name), move(index_variable_name), move(iterated_expression), move(syntax_error), move(in_start_position), move(index_start_position)); // ForLoop Var Iterated Block } } @@ -697,7 +739,7 @@ RefPtr Parser::parse_for_loop() auto cbrace_error_start = push_start(); if (!expect('}')) { auto error_start = push_start(); - auto syntax_error = create("Expected a close brace '}' to end a 'for' loop body"_string.release_value_but_fixme_should_propagate_errors(), true); + auto syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a 'for' loop body"_string), true); if (body) body->set_is_syntax_error(*syntax_error); else @@ -722,7 +764,7 @@ RefPtr Parser::parse_loop_loop() { auto obrace_error_start = push_start(); if (!expect('{')) { - auto syntax_error = create("Expected an open brace '{' to start a 'loop' loop body"_string.release_value_but_fixme_should_propagate_errors(), true); + auto syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start a 'loop' loop body"_string), true); return create(AST::NameWithPosition {}, AST::NameWithPosition {}, nullptr, move(syntax_error)); // ForLoop null null Block } } @@ -734,7 +776,7 @@ RefPtr Parser::parse_loop_loop() auto cbrace_error_start = push_start(); if (!expect('}')) { auto error_start = push_start(); - auto syntax_error = create("Expected a close brace '}' to end a 'loop' loop body"_string.release_value_but_fixme_should_propagate_errors(), true); + auto syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a 'loop' loop body"_string), true); if (body) body->set_is_syntax_error(*syntax_error); else @@ -761,7 +803,7 @@ RefPtr Parser::parse_if_expr() auto cond_error_start = push_start(); condition = parse_or_logical_sequence(); if (!condition) - condition = create("Expected a logical sequence after 'if'"_string.release_value_but_fixme_should_propagate_errors(), true); + condition = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a logical sequence after 'if'"_string), true); } auto parse_braced_toplevel = [&]() -> RefPtr { @@ -769,7 +811,7 @@ RefPtr Parser::parse_if_expr() { auto obrace_error_start = push_start(); if (!expect('{')) { - body = create("Expected an open brace '{' to start an 'if' true branch"_string.release_value_but_fixme_should_propagate_errors(), true); + body = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start an 'if' true branch"_string), true); } } @@ -780,7 +822,7 @@ RefPtr Parser::parse_if_expr() auto cbrace_error_start = push_start(); if (!expect('}')) { auto error_start = push_start(); - RefPtr syntax_error = create("Expected a close brace '}' to end an 'if' true branch"_string.release_value_but_fixme_should_propagate_errors(), true); + RefPtr syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end an 'if' true branch"_string), true); if (body) body->set_is_syntax_error(*syntax_error); else @@ -832,7 +874,7 @@ RefPtr Parser::parse_subshell() auto cbrace_error_start = push_start(); if (!expect('}')) { auto error_start = push_start(); - RefPtr syntax_error = create("Expected a close brace '}' to end a subshell"_string.release_value_but_fixme_should_propagate_errors(), true); + RefPtr syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a subshell"_string), true); if (body) body->set_is_syntax_error(*syntax_error); else @@ -857,7 +899,7 @@ RefPtr Parser::parse_match_expr() auto match_expression = parse_expression(); if (!match_expression) { return create( - create("Expected an expression after 'match'"_string.release_value_but_fixme_should_propagate_errors(), true), + create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an expression after 'match'"_string), true), String {}, Optional {}, Vector {}); } @@ -874,16 +916,16 @@ RefPtr Parser::parse_match_expr() auto node = create( match_expression.release_nonnull(), String {}, move(as_position), Vector {}); - node->set_is_syntax_error(create("Expected whitespace after 'as' in 'match'"_string.release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected whitespace after 'as' in 'match'"_string), true)); return node; } - match_name = String::from_utf8(consume_while(is_word_character)).release_value_but_fixme_should_propagate_errors(); + match_name = TRY_OR_THROW_PARSE_ERROR(String::from_utf8(consume_while(is_word_character))); if (match_name.is_empty()) { auto node = create( match_expression.release_nonnull(), String {}, move(as_position), Vector {}); - node->set_is_syntax_error(create("Expected an identifier after 'as' in 'match'"_string.release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an identifier after 'as' in 'match'"_string), true)); return node; } } @@ -894,7 +936,7 @@ RefPtr Parser::parse_match_expr() auto node = create( match_expression.release_nonnull(), move(match_name), move(as_position), Vector {}); - node->set_is_syntax_error(create("Expected an open brace '{' to start a 'match' entry list"_string.release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start a 'match' entry list"_string), true)); return node; } @@ -916,7 +958,7 @@ RefPtr Parser::parse_match_expr() auto node = create( match_expression.release_nonnull(), move(match_name), move(as_position), move(entries)); - node->set_is_syntax_error(create("Expected a close brace '}' to end a 'match' entry list"_string.release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a 'match' entry list"_string), true)); return node; } @@ -942,14 +984,14 @@ AST::MatchEntry Parser::parse_match_entry() auto regex_pattern = parse_regex_pattern(); if (regex_pattern.has_value()) { if (auto error = regex_pattern.value().parser_result.error; error != regex::Error::NoError) - return { Vector> {}, {}, {}, {}, create(String::from_utf8(regex::get_error_string(error)).release_value_but_fixme_should_propagate_errors(), false) }; + return { Vector> {}, {}, {}, {}, create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::from_utf8(regex::get_error_string(error))), false) }; pattern_kind = Regex; regexps.append(regex_pattern.release_value()); } else { auto glob_pattern = parse_match_pattern(); if (!glob_pattern) - return { Vector> {}, {}, {}, {}, create("Expected a pattern in 'match' body"_string.release_value_but_fixme_should_propagate_errors(), true) }; + return { Vector> {}, {}, {}, {}, create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a pattern in 'match' body"_string), true) }; pattern_kind = Glob; patterns.append(glob_pattern.release_nonnull()); @@ -967,7 +1009,7 @@ AST::MatchEntry Parser::parse_match_entry() case Regex: { auto pattern = parse_regex_pattern(); if (!pattern.has_value()) { - error = create("Expected a regex pattern to follow '|' in 'match' body"_string.release_value_but_fixme_should_propagate_errors(), true); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a regex pattern to follow '|' in 'match' body"_string), true); break; } regexps.append(pattern.release_value()); @@ -976,7 +1018,7 @@ AST::MatchEntry Parser::parse_match_entry() case Glob: { auto pattern = parse_match_pattern(); if (!pattern) { - error = create("Expected a pattern to follow '|' in 'match' body"_string.release_value_but_fixme_should_propagate_errors(), true); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a pattern to follow '|' in 'match' body"_string), true); break; } patterns.append(pattern.release_nonnull()); @@ -999,7 +1041,7 @@ AST::MatchEntry Parser::parse_match_entry() consume_while(is_any_of(" \t\n"sv)); if (!expect('(')) { if (!error) - error = create("Expected an explicit list of identifiers after a pattern 'as'"_string.release_value_but_fixme_should_propagate_errors()); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an explicit list of identifiers after a pattern 'as'"_string)); } else { match_names = Vector(); for (;;) { @@ -1007,12 +1049,15 @@ AST::MatchEntry Parser::parse_match_entry() auto name = consume_while(is_word_character); if (name.is_empty()) break; - match_names->append(String::from_utf8(name).release_value_but_fixme_should_propagate_errors()); + match_names->append(TRY_OR( + String::from_utf8(name), + error = create(MUST(String::from_utf8(_error.string_literal()))); + break;)); } if (!expect(')')) { if (!error) - error = create("Expected a close paren ')' to end the identifier list of pattern 'as'"_string.release_value_but_fixme_should_propagate_errors(), true); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close paren ')' to end the identifier list of pattern 'as'"_string), true); } } consume_while(is_any_of(" \t\n"sv)); @@ -1023,18 +1068,24 @@ AST::MatchEntry Parser::parse_match_entry() for (auto& regex : regexps) { if (names.is_empty()) { for (auto& name : regex.parser_result.capture_groups) - names.append(String::from_deprecated_string(name).release_value_but_fixme_should_propagate_errors()); + names.append(TRY_OR( + String::from_deprecated_string(name), + error = create(MUST(String::from_utf8(_error.string_literal()))); + break;)); } else { size_t index = 0; for (auto& name : regex.parser_result.capture_groups) { if (names.size() <= index) { - names.append(String::from_deprecated_string(name).release_value_but_fixme_should_propagate_errors()); + names.append(TRY_OR( + String::from_deprecated_string(name), + error = create(MUST(String::from_utf8(_error.string_literal()))); + break;)); continue; } if (names[index] != name.view()) { if (!error) - error = create("Alternative regex patterns must have the same capture groups"_string.release_value_but_fixme_should_propagate_errors(), false); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Alternative regex patterns must have the same capture groups"_string), false); break; } } @@ -1045,14 +1096,14 @@ AST::MatchEntry Parser::parse_match_entry() if (!expect('{')) { if (!error) - error = create("Expected an open brace '{' to start a match entry body"_string.release_value_but_fixme_should_propagate_errors(), true); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an open brace '{' to start a match entry body"_string), true); } auto body = parse_toplevel(); if (!expect('}')) { if (!error) - error = create("Expected a close brace '}' to end a match entry body"_string.release_value_but_fixme_should_propagate_errors(), true); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a match entry body"_string), true); } if (body && error) @@ -1131,7 +1182,7 @@ RefPtr Parser::parse_redirection() // Eat a character and hope the problem goes away consume(); } - path = create("Expected a path after redirection"_string.release_value_but_fixme_should_propagate_errors(), true); + path = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a path after redirection"_string), true); } return create(pipe_fd, path.release_nonnull()); // Redirection WriteAppend } @@ -1154,7 +1205,7 @@ RefPtr Parser::parse_redirection() } auto redir = create(pipe_fd, dest_pipe_fd); // Redirection Fd2Fd if (dest_pipe_fd == -1) - redir->set_is_syntax_error(*create("Expected a file descriptor"_string.release_value_but_fixme_should_propagate_errors())); + redir->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a file descriptor"_string))); return redir; } consume_while(is_whitespace); @@ -1165,7 +1216,7 @@ RefPtr Parser::parse_redirection() // Eat a character and hope the problem goes away consume(); } - path = create("Expected a path after redirection"_string.release_value_but_fixme_should_propagate_errors(), true); + path = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a path after redirection"_string), true); } return create(pipe_fd, path.release_nonnull()); // Redirection Write } @@ -1189,7 +1240,7 @@ RefPtr Parser::parse_redirection() // Eat a character and hope the problem goes away consume(); } - path = create("Expected a path after redirection"_string.release_value_but_fixme_should_propagate_errors(), true); + path = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a path after redirection"_string), true); } if (mode == Read) return create(pipe_fd, path.release_nonnull()); // Redirection Read @@ -1226,7 +1277,7 @@ RefPtr Parser::parse_expression() { auto rule_start = push_start(); if (m_rule_start_offsets.size() > max_allowed_nested_rule_depth) - return create(String::formatted("Expression nested too deep (max allowed is {})", max_allowed_nested_rule_depth).release_value_but_fixme_should_propagate_errors()); + return create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expression nested too deep (max allowed is {})", max_allowed_nested_rule_depth))); auto starting_char = peek(); @@ -1362,10 +1413,10 @@ RefPtr Parser::parse_string() consume(); auto inner = parse_string_inner(StringEndCondition::DoubleQuote); if (!inner) - inner = create("Unexpected EOF in string"_string.release_value_but_fixme_should_propagate_errors(), true); + inner = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Unexpected EOF in string"_string), true); if (!expect('"')) { inner = create(move(inner)); - inner->set_is_syntax_error(*create("Expected a terminating double quote"_string.release_value_but_fixme_should_propagate_errors(), true)); + inner->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a terminating double quote"_string), true)); return inner; } return create(move(inner)); // Double Quoted String @@ -1377,9 +1428,9 @@ RefPtr Parser::parse_string() bool is_error = false; if (!expect('\'')) is_error = true; - auto result = create(String::from_utf8(text).release_value_but_fixme_should_propagate_errors(), AST::StringLiteral::EnclosureType::SingleQuotes); // String Literal + auto result = create(TRY_OR_THROW_PARSE_ERROR(String::from_utf8(text)), AST::StringLiteral::EnclosureType::SingleQuotes); // String Literal if (is_error) - result->set_is_syntax_error(*create("Expected a terminating single quote"_string.release_value_but_fixme_should_propagate_errors(), true)); + result->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a terminating single quote"_string), true)); return result; } @@ -1459,7 +1510,7 @@ RefPtr Parser::parse_string_inner(StringEndCondition condition) continue; } if (peek() == '$') { - auto string_literal = create(builder.to_string().release_value_but_fixme_should_propagate_errors(), AST::StringLiteral::EnclosureType::DoubleQuotes); // String Literal + auto string_literal = create(TRY_OR_THROW_PARSE_ERROR(builder.to_string()), AST::StringLiteral::EnclosureType::DoubleQuotes); // String Literal auto read_concat = [&](auto&& node) { auto inner = create( move(string_literal), @@ -1485,7 +1536,7 @@ RefPtr Parser::parse_string_inner(StringEndCondition condition) builder.append(consume()); } - return create(builder.to_string().release_value_but_fixme_should_propagate_errors(), AST::StringLiteral::EnclosureType::DoubleQuotes); // String Literal + return create(TRY_OR_THROW_PARSE_ERROR(builder.to_string()), AST::StringLiteral::EnclosureType::DoubleQuotes); // String Literal } RefPtr Parser::parse_variable() @@ -1530,7 +1581,7 @@ RefPtr Parser::parse_variable_ref() return nullptr; } - return create(String::from_utf8(name).release_value_but_fixme_should_propagate_errors()); // Variable Simple + return create(TRY_OR_THROW_PARSE_ERROR(String::from_utf8(name))); // Variable Simple } RefPtr Parser::parse_slice() @@ -1548,7 +1599,7 @@ RefPtr Parser::parse_slice() RefPtr error; if (peek() != ']') - error = create("Expected a close bracket ']' to end a variable slice"_string.release_value_but_fixme_should_propagate_errors()); + error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close bracket ']' to end a variable slice"_string)); else consume(); @@ -1556,7 +1607,7 @@ RefPtr Parser::parse_slice() if (error) spec = move(error); else - spec = create("Expected either a range, or a comma-seprated list of selectors"_string.release_value_but_fixme_should_propagate_errors()); + spec = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected either a range, or a comma-seprated list of selectors"_string)); } auto node = create(spec.release_nonnull()); @@ -1579,16 +1630,16 @@ RefPtr Parser::parse_evaluate() consume(); auto inner = parse_pipe_sequence(); if (!inner) - inner = create("Unexpected EOF in list"_string.release_value_but_fixme_should_propagate_errors(), true); + inner = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Unexpected EOF in list"_string), true); if (!expect(')')) - inner->set_is_syntax_error(*create("Expected a terminating close paren"_string.release_value_but_fixme_should_propagate_errors(), true)); + inner->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a terminating close paren"_string), true)); return create(inner.release_nonnull(), true); } auto inner = parse_expression(); if (!inner) { - inner = create("Expected a command"_string.release_value_but_fixme_should_propagate_errors(), true); + inner = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a command"_string), true); } else { if (inner->is_list()) { auto execute_inner = create(inner.release_nonnull(), true); @@ -1659,14 +1710,14 @@ RefPtr Parser::parse_immediate_expression() }; auto node = create( - AST::NameWithPosition { String::from_utf8(function_name).release_value_but_fixme_should_propagate_errors(), move(function_position) }, + AST::NameWithPosition { TRY_OR_THROW_PARSE_ERROR(String::from_utf8(function_name)), move(function_position) }, move(arguments), ending_brace_position); if (!ending_brace_position.has_value()) - node->set_is_syntax_error(create("Expected a closing brace '}' to end an immediate expression"_string.release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a closing brace '}' to end an immediate expression"_string), true)); else if (node->function_name().is_empty()) - node->set_is_syntax_error(create("Expected an immediate function name"_string.release_value_but_fixme_should_propagate_errors())); + node->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an immediate function name"_string))); return node; } @@ -1744,7 +1795,7 @@ RefPtr Parser::parse_history_designator() if (number != 0) selector.event.index = number - 1; else - syntax_error = create("History entry index value invalid or out of range"_string.release_value_but_fixme_should_propagate_errors()); + syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("History entry index value invalid or out of range"_string)); } if (":^$*"sv.contains(peek())) { is_word_selector = true; @@ -1804,7 +1855,7 @@ RefPtr Parser::parse_history_designator() auto first_char = peek(); if (!(is_digit(first_char) || "^$-*"sv.contains(first_char))) { if (!syntax_error) - syntax_error = create("Expected a word selector after ':' in a history event designator"_string.release_value_but_fixme_should_propagate_errors(), true); + syntax_error = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a word selector after ':' in a history event designator"_string), true); } else if (first_char == '*') { consume(); selector.word_selector_range.start = make_word_selector(AST::HistorySelector::WordSelectorKind::Index, 1); @@ -1854,7 +1905,7 @@ RefPtr Parser::parse_comment() consume(); auto text = consume_while(is_not('\n')); - return create(String::from_utf8(text).release_value_but_fixme_should_propagate_errors()); // Comment + return create(TRY_OR_THROW_PARSE_ERROR(String::from_utf8(text))); // Comment } RefPtr Parser::parse_bareword() @@ -1896,17 +1947,17 @@ RefPtr Parser::parse_bareword() auto current_end = m_offset; auto current_line = line(); - auto string = builder.to_string().release_value_but_fixme_should_propagate_errors(); + auto string = TRY_OR_THROW_PARSE_ERROR(builder.to_string()); if (string.starts_with('~')) { String username; RefPtr tilde, text; auto first_slash_index = string.find_byte_offset('/'); if (first_slash_index.has_value()) { - username = string.substring_from_byte_offset(1, *first_slash_index - 1).release_value_but_fixme_should_propagate_errors(); - string = string.substring_from_byte_offset(*first_slash_index).release_value_but_fixme_should_propagate_errors(); + username = TRY_OR_THROW_PARSE_ERROR(string.substring_from_byte_offset(1, *first_slash_index - 1)); + string = TRY_OR_THROW_PARSE_ERROR(string.substring_from_byte_offset(*first_slash_index)); } else { - username = string.substring_from_byte_offset(1).release_value_but_fixme_should_propagate_errors(); + username = TRY_OR_THROW_PARSE_ERROR(string.substring_from_byte_offset(1)); string = {}; } @@ -1937,7 +1988,7 @@ RefPtr Parser::parse_bareword() if (string.starts_with_bytes("\\~"sv)) { // Un-escape the tilde, but only at the start (where it would be an expansion) - string = string.substring_from_byte_offset(1).release_value_but_fixme_should_propagate_errors(); + string = TRY_OR_THROW_PARSE_ERROR(string.substring_from_byte_offset(1)); } return create(move(string)); // Bareword Literal @@ -1964,7 +2015,7 @@ RefPtr Parser::parse_glob() } else { // FIXME: Allow composition of tilde+bareword with globs: '~/foo/bar/baz*' restore_to(saved_offset.offset, saved_offset.line); - bareword_part->set_is_syntax_error(*create(String::formatted("Unexpected {} inside a glob", bareword_part->class_name()).release_value_but_fixme_should_propagate_errors())); + bareword_part->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Unexpected {} inside a glob", bareword_part->class_name())))); return bareword_part; } textbuilder.append(text); @@ -1985,11 +2036,11 @@ RefPtr Parser::parse_glob() textbuilder.append('~'); textbuilder.append(bareword->text()); } else { - return create(String::formatted("Invalid node '{}' in glob position, escape shell special characters", glob_after->class_name()).release_value_but_fixme_should_propagate_errors()); + return create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Invalid node '{}' in glob position, escape shell special characters", glob_after->class_name()))); } } - return create(textbuilder.to_string().release_value_but_fixme_should_propagate_errors()); // Glob + return create(TRY_OR_THROW_PARSE_ERROR(textbuilder.to_string())); // Glob } return bareword_part; @@ -2004,7 +2055,7 @@ RefPtr Parser::parse_brace_expansion() if (auto spec = parse_brace_expansion_spec()) { if (!expect('}')) - spec->set_is_syntax_error(create("Expected a close brace '}' to end a brace expansion"_string.release_value_but_fixme_should_propagate_errors(), true)); + spec->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a close brace '}' to end a brace expansion"_string), true)); return spec; } @@ -2032,12 +2083,12 @@ RefPtr Parser::parse_brace_expansion_spec() if (expect(".."sv)) { if (auto end_expr = parse_expression()) { if (end_expr->position().start_offset != start_expr->position().end_offset + 2) - end_expr->set_is_syntax_error(create("Expected no whitespace between '..' and the following expression in brace expansion"_string.release_value_but_fixme_should_propagate_errors())); + end_expr->set_is_syntax_error(create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected no whitespace between '..' and the following expression in brace expansion"_string))); return create(start_expr.release_nonnull(), end_expr.release_nonnull()); } - return create(start_expr.release_nonnull(), create("Expected an expression to end range brace expansion with"_string.release_value_but_fixme_should_propagate_errors(), true)); + return create(start_expr.release_nonnull(), create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected an expression to end range brace expansion with"_string), true)); } } @@ -2072,7 +2123,7 @@ RefPtr Parser::parse_heredoc_initiation_record() consume(); HeredocInitiationRecord record; - record.end = ""_string.release_value_but_fixme_should_propagate_errors(); + record.end = TRY_OR_THROW_PARSE_ERROR(""_string); RefPtr syntax_error_node; @@ -2094,7 +2145,7 @@ RefPtr Parser::parse_heredoc_initiation_record() // StringLiteral | bareword if (auto bareword = parse_bareword()) { if (!bareword->is_bareword()) { - syntax_error_node = create(String::formatted("Expected a bareword or a quoted string, not {}", bareword->class_name()).release_value_but_fixme_should_propagate_errors()); + syntax_error_node = create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expected a bareword or a quoted string, not {}", bareword->class_name()))); } else { if (bareword->is_syntax_error()) syntax_error_node = bareword->syntax_error_node(); @@ -2110,19 +2161,19 @@ RefPtr Parser::parse_heredoc_initiation_record() if (!expect('\'')) is_error = true; if (is_error) - syntax_error_node = create("Expected a terminating single quote"_string.release_value_but_fixme_should_propagate_errors(), true); + syntax_error_node = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a terminating single quote"_string), true); - record.end = String::from_utf8(text).release_value_but_fixme_should_propagate_errors(); + record.end = TRY_OR_THROW_PARSE_ERROR(String::from_utf8(text)); record.interpolate = false; } else { - syntax_error_node = create("Expected a bareword or a single-quoted string literal for heredoc end key"_string.release_value_but_fixme_should_propagate_errors(), true); + syntax_error_node = create(TRY_OR_RESOLVE_TO_ERROR_STRING("Expected a bareword or a single-quoted string literal for heredoc end key"_string), true); } auto node = create(record.end, record.interpolate, record.deindent); if (syntax_error_node) node->set_is_syntax_error(*syntax_error_node); else - node->set_is_syntax_error(*create(String::formatted("Expected heredoc contents for heredoc with end key '{}'", node->end()).release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expected heredoc contents for heredoc with end key '{}'", node->end())), true)); record.node = node; m_heredoc_initiations.append(move(record)); @@ -2138,7 +2189,7 @@ bool Parser::parse_heredoc_entries() for (auto& record : heredocs) { auto rule_start = push_start(); if (m_rule_start_offsets.size() > max_allowed_nested_rule_depth) { - record.node->set_is_syntax_error(*create(String::formatted("Expression nested too deep (max allowed is {})", max_allowed_nested_rule_depth).release_value_but_fixme_should_propagate_errors())); + record.node->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expression nested too deep (max allowed is {})", max_allowed_nested_rule_depth)))); continue; } bool found_key = false; @@ -2164,10 +2215,10 @@ bool Parser::parse_heredoc_entries() last_line_offset = current_position(); // Now just wrap it in a StringLiteral and set it as the node's contents auto node = create( - String::from_utf8(m_input.substring_view(rule_start->offset, last_line_offset->offset - rule_start->offset)).release_value_but_fixme_should_propagate_errors(), + MUST(String::from_utf8(m_input.substring_view(rule_start->offset, last_line_offset->offset - rule_start->offset))), AST::StringLiteral::EnclosureType::None); if (!found_key) - node->set_is_syntax_error(*create(String::formatted("Expected to find the heredoc key '{}', but found Eof", record.end).release_value_but_fixme_should_propagate_errors(), true)); + node->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expected to find the heredoc key '{}', but found Eof", record.end)), true)); record.node->set_contents(move(node)); } else { // Interpolation is allowed, so we're going to read doublequoted string innards @@ -2216,9 +2267,9 @@ bool Parser::parse_heredoc_entries() if (!expr && found_key) { expr = create(String {}, AST::StringLiteral::EnclosureType::None); } else if (!expr) { - expr = create(String::formatted("Expected to find a valid string inside a heredoc (with end key '{}')", record.end).release_value_but_fixme_should_propagate_errors(), true); + expr = create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expected to find a valid string inside a heredoc (with end key '{}')", record.end)), true); } else if (!found_key) { - expr->set_is_syntax_error(*create(String::formatted("Expected to find the heredoc key '{}'", record.end).release_value_but_fixme_should_propagate_errors(), true)); + expr->set_is_syntax_error(*create(TRY_OR_RESOLVE_TO_ERROR_STRING(String::formatted("Expected to find the heredoc key '{}'", record.end)), true)); } record.node->set_contents(create(move(expr))); diff --git a/Userland/Shell/PosixParser.cpp b/Userland/Shell/PosixParser.cpp index 59923fb4e7..3594f59904 100644 --- a/Userland/Shell/PosixParser.cpp +++ b/Userland/Shell/PosixParser.cpp @@ -9,6 +9,20 @@ #include #include +#define TRY_OR_THROW_PARSE_ERROR_AT(expr, position) ({ \ + /* Ignore -Wshadow to allow nesting the macro. */ \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto&& _value_or_error = expr;) \ + if (_value_or_error.is_error()) { \ + AK_IGNORE_DIAGNOSTIC("-Wshadow", \ + auto _error = _value_or_error.release_error();) \ + if (_error.is_errno() && _error.code() == ENOMEM) \ + return make_ref_counted(position, "OOM"_short_string); \ + return make_ref_counted(position, MUST(String::formatted("Error: {}", _error))); \ + } \ + _value_or_error.release_value(); \ +}) + static Shell::AST::Position empty_position() { return { 0, 0, { 0, 0 }, { 0, 0 } }; @@ -132,7 +146,8 @@ ErrorOr Parser::fill_token_buffer(Optional starting_reduction) RefPtr Parser::parse() { - return parse_complete_command().release_value_but_fixme_should_propagate_errors(); + auto start_position = peek().position.value_or(empty_position()); + return TRY_OR_THROW_PARSE_ERROR_AT(parse_complete_command(), start_position); } void Parser::handle_heredoc_contents() @@ -147,16 +162,26 @@ void Parser::handle_heredoc_contents() auto& heredoc = **entry; - RefPtr contents; - if (heredoc.allow_interpolation()) { - Parser parser { token.value, m_in_interactive_mode, Reduction::HeredocContents }; - contents = parser.parse_word().release_value_but_fixme_should_propagate_errors(); - } else { - contents = make_ref_counted(token.position.value_or(empty_position()), String::from_utf8(token.value).release_value_but_fixme_should_propagate_errors(), AST::StringLiteral::EnclosureType::None); + auto contents_or_error = [&]() -> ErrorOr> { + if (heredoc.allow_interpolation()) { + Parser parser { token.value, m_in_interactive_mode, Reduction::HeredocContents }; + return parser.parse_word(); + } + + return make_ref_counted( + token.position.value_or(empty_position()), + TRY(String::from_utf8(token.value)), + AST::StringLiteral::EnclosureType::None); + }(); + + if (contents_or_error.is_error()) { + warnln("Shell: Failed to parse heredoc contents: {}", contents_or_error.error()); + continue; } - if (contents) + if (auto contents = contents_or_error.release_value()) heredoc.set_contents(contents); + m_unprocessed_heredoc_entries.remove(*token.relevant_heredoc_key); } } @@ -639,7 +664,7 @@ ErrorOr> Parser::parse_complete_command() auto position = peek().position; auto syntax_error = make_ref_counted( position.value_or(empty_position()), - "Extra tokens after complete command"_string.release_value_but_fixme_should_propagate_errors()); + TRY("Extra tokens after complete command"_string)); if (list) list->set_is_syntax_error(*syntax_error); @@ -836,7 +861,7 @@ ErrorOr> Parser::parse_function_definition() return make_ref_counted( name.position.value_or(empty_position()).with_end(peek().position.value_or(empty_position())), - AST::NameWithPosition { String::from_utf8(name.value).release_value_but_fixme_should_propagate_errors(), name.position.value_or(empty_position()) }, + AST::NameWithPosition { TRY(String::from_utf8(name.value)), name.position.value_or(empty_position()) }, Vector {}, body.release_nonnull()); } @@ -920,13 +945,13 @@ ErrorOr> Parser::parse_while_clause() if (!condition) condition = make_ref_counted( peek().position.value_or(empty_position()), - "Expected condition after 'while'"_string.release_value_but_fixme_should_propagate_errors()); + TRY("Expected condition after 'while'"_string)); auto do_group = TRY(parse_do_group()); if (!do_group) do_group = make_ref_counted( peek().position.value_or(empty_position()), - "Expected 'do' after 'while'"_string.release_value_but_fixme_should_propagate_errors()); + TRY("Expected 'do' after 'while'"_string)); // while foo; bar -> loop { if foo { bar } else { break } } auto position = start_position.with_end(peek().position.value_or(empty_position())); @@ -956,13 +981,13 @@ ErrorOr> Parser::parse_until_clause() if (!condition) condition = make_ref_counted( peek().position.value_or(empty_position()), - "Expected condition after 'until'"_string.release_value_but_fixme_should_propagate_errors()); + TRY("Expected condition after 'until'"_string)); auto do_group = TRY(parse_do_group()); if (!do_group) do_group = make_ref_counted( peek().position.value_or(empty_position()), - "Expected 'do' after 'until'"_string.release_value_but_fixme_should_propagate_errors()); + TRY("Expected 'do' after 'until'"_string)); // until foo; bar -> loop { if foo { break } else { bar } } auto position = start_position.with_end(peek().position.value_or(empty_position())); @@ -995,7 +1020,7 @@ ErrorOr> Parser::parse_brace_group() if (peek().type != Token::Type::CloseBrace) { error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected '}}', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected '}}', not {}", peek().type_name()))); } else { consume(); } @@ -1023,12 +1048,12 @@ ErrorOr> Parser::parse_case_clause() if (!expr) expr = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected a word, not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected a word, not {}", peek().type_name()))); if (peek().type != Token::Type::In) { syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'in', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'in', not {}", peek().type_name()))); } else { skip(); } @@ -1062,7 +1087,7 @@ ErrorOr> Parser::parse_case_clause() if (!syntax_error) syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected ')', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected ')', not {}", peek().type_name()))); break; } @@ -1077,7 +1102,7 @@ ErrorOr> Parser::parse_case_clause() if (!syntax_error) syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected ';;', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected ';;', not {}", peek().type_name()))); } if (syntax_error) { @@ -1101,7 +1126,7 @@ ErrorOr> Parser::parse_case_clause() if (peek().type != Token::Type::Esac) { syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'esac', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'esac', not {}", peek().type_name()))); } else { skip(); } @@ -1136,7 +1161,7 @@ ErrorOr Parser::parse_case_list() if (!node) node = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected a word, not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected a word, not {}", peek().type_name()))); nodes.append(node.release_nonnull()); @@ -1151,7 +1176,7 @@ ErrorOr Parser::parse_case_list() if (nodes.is_empty()) nodes.append(make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected a word, not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors())); + TRY(String::formatted("Expected a word, not {}", peek().type_name())))); return CaseItemsResult { move(pipes), move(nodes) }; } @@ -1166,20 +1191,20 @@ ErrorOr> Parser::parse_if_clause() skip(); auto main_condition = TRY(parse_compound_list()); if (!main_condition) - main_condition = make_ref_counted(empty_position(), "Expected compound list after 'if'"_string.release_value_but_fixme_should_propagate_errors()); + main_condition = make_ref_counted(empty_position(), TRY("Expected compound list after 'if'"_string)); RefPtr syntax_error; if (peek().type != Token::Type::Then) { syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'then', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'then', not {}", peek().type_name()))); } else { skip(); } auto main_consequence = TRY(parse_compound_list()); if (!main_consequence) - main_consequence = make_ref_counted(empty_position(), "Expected compound list after 'then'"_string.release_value_but_fixme_should_propagate_errors()); + main_consequence = make_ref_counted(empty_position(), TRY("Expected compound list after 'then'"_string)); auto node = make_ref_counted(start_position, Optional(), main_condition.release_nonnull(), main_consequence.release_nonnull(), nullptr); auto active_node = node; @@ -1188,20 +1213,20 @@ ErrorOr> Parser::parse_if_clause() skip(); auto condition = TRY(parse_compound_list()); if (!condition) - condition = make_ref_counted(empty_position(), "Expected compound list after 'elif'"_string.release_value_but_fixme_should_propagate_errors()); + condition = make_ref_counted(empty_position(), TRY("Expected compound list after 'elif'"_string)); if (peek().type != Token::Type::Then) { if (!syntax_error) syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'then', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'then', not {}", peek().type_name()))); } else { skip(); } auto consequence = TRY(parse_compound_list()); if (!consequence) - consequence = make_ref_counted(empty_position(), "Expected compound list after 'then'"_string.release_value_but_fixme_should_propagate_errors()); + consequence = make_ref_counted(empty_position(), TRY("Expected compound list after 'then'"_string)); auto new_node = make_ref_counted(start_position, Optional(), condition.release_nonnull(), consequence.release_nonnull(), nullptr); @@ -1215,7 +1240,7 @@ ErrorOr> Parser::parse_if_clause() skip(); active_node->false_branch() = TRY(parse_compound_list()); if (!active_node->false_branch()) - active_node->false_branch() = make_ref_counted(empty_position(), "Expected compound list after 'else'"_string.release_value_but_fixme_should_propagate_errors()); + active_node->false_branch() = make_ref_counted(empty_position(), TRY("Expected compound list after 'else'"_string)); break; case Token::Type::Fi: skip(); @@ -1225,7 +1250,7 @@ ErrorOr> Parser::parse_if_clause() if (!syntax_error) syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'else' or 'fi', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'else' or 'fi', not {}", peek().type_name()))); break; } @@ -1234,7 +1259,7 @@ ErrorOr> Parser::parse_if_clause() if (!syntax_error) syntax_error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'fi', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'fi', not {}", peek().type_name()))); } else { skip(); } @@ -1257,10 +1282,10 @@ ErrorOr> Parser::parse_subshell() auto list = TRY(parse_compound_list()); if (!list) - error = make_ref_counted(peek().position.value_or(empty_position()), "Expected compound list after ("_string.release_value_but_fixme_should_propagate_errors()); + error = make_ref_counted(peek().position.value_or(empty_position()), TRY("Expected compound list after ("_string)); if (peek().type != Token::Type::CloseParen) - error = make_ref_counted(peek().position.value_or(empty_position()), "Expected ) after compound list"_string.release_value_but_fixme_should_propagate_errors()); + error = make_ref_counted(peek().position.value_or(empty_position()), TRY("Expected ) after compound list"_string)); else skip(); @@ -1392,7 +1417,7 @@ RefPtr Parser::parse_word_list() auto start_position = peek().position.value_or(empty_position()); for (; peek().type == Token::Type::Word;) { - auto word = parse_word().release_value_but_fixme_should_propagate_errors(); + auto word = TRY_OR_THROW_PARSE_ERROR_AT(parse_word(), start_position); nodes.append(word.release_nonnull()); } @@ -1809,7 +1834,7 @@ ErrorOr> Parser::parse_do_group() if (peek().type != Token::Type::Do) { return make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'do', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'do', not {}", peek().type_name()))); } consume(); @@ -1820,7 +1845,7 @@ ErrorOr> Parser::parse_do_group() if (peek().type != Token::Type::Done) { error = make_ref_counted( peek().position.value_or(empty_position()), - String::formatted("Expected 'done', not {}", peek().type_name()).release_value_but_fixme_should_propagate_errors()); + TRY(String::formatted("Expected 'done', not {}", peek().type_name()))); } else { consume(); } @@ -1986,7 +2011,7 @@ ErrorOr> Parser::parse_io_here(AST::Position start_position, O auto end_keyword = consume(); if (!is_one_of(end_keyword.type, Token::Type::Word, Token::Type::Token)) - return make_ref_counted(io_operator_token.position.value_or(start_position), "Expected a heredoc keyword"_string.release_value_but_fixme_should_propagate_errors(), true); + return make_ref_counted(io_operator_token.position.value_or(start_position), TRY("Expected a heredoc keyword"_string), true); auto [end_keyword_text, allow_interpolation] = Lexer::process_heredoc_key(end_keyword); RefPtr error;