From 1e1d9b0cb6207595abb4216082edd4ea56040a43 Mon Sep 17 00:00:00 2001 From: Robert Bendun Date: Thu, 2 Jun 2022 21:39:22 +0200 Subject: [PATCH] Error reporting of use of non-symbols in indentifier context --- src/errors.cc | 17 ++++++++++++- src/musique.hh | 9 +++++++ src/parser.cc | 69 +++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 8 deletions(-) diff --git a/src/errors.cc b/src/errors.cc index dafe3b8..fb4c624 100644 --- a/src/errors.cc +++ b/src/errors.cc @@ -119,7 +119,8 @@ std::ostream& operator<<(std::ostream& os, Error const& err) [](errors::Unexpected_Keyword const&) { return "Unexpected keyword"; }, [](errors::Unrecognized_Character const&) { return "Unrecognized character"; }, [](errors::internal::Unexpected_Token const&) { return "Unexpected token"; }, - [](errors::Expected_Expression_Separator_Before const&) { return "Missing semicolon"; } + [](errors::Expected_Expression_Separator_Before const&) { return "Missing semicolon"; }, + [](errors::Literal_As_Identifier const&) { return "Literal used in place of an identifier"; } }, err.details); error_heading(os, err.location, Error_Level::Error, short_description); @@ -161,6 +162,20 @@ std::ostream& operator<<(std::ostream& os, Error const& err) } }, + [&os](errors::Literal_As_Identifier const& err) { + os << "I expected an identifier in " << err.context << ", but found" << (err.type_name.empty() ? "" : " ") << err.type_name << " value = '" << err.source << "'\n"; + + + if (err.type_name == "chord") { + os << "\nTry renaming to different name or appending with something that is not part of chord literal like 'x'\n"; + + os << pretty::begin_comment << + "\nMusical notation names are reserved for chord and note notations,\n" + "and cannot be reused as an identifier to prevent ambiguity\n" + << pretty::end; + } + }, + [&os](errors::Not_Callable const&) { unimplemented(); }, [&os](errors::Undefined_Operator const&) { unimplemented(); }, [&os](errors::Unexpected_Keyword const&) { unimplemented(); }, diff --git a/src/musique.hh b/src/musique.hh index 29be61f..fdd1697 100644 --- a/src/musique.hh +++ b/src/musique.hh @@ -83,6 +83,14 @@ namespace errors std::string_view type; }; + // When user provides literal where identifier should be + struct Literal_As_Identifier + { + std::string_view type_name; + std::string_view source; + std::string_view context; + }; + /// Collection of messages that are considered internal and should not be printed to the end user. namespace internal { @@ -109,6 +117,7 @@ namespace errors Unexpected_Empty_Source, Unexpected_Keyword, Unrecognized_Character, + Literal_As_Identifier, internal::Unexpected_Token >; } diff --git a/src/parser.cc b/src/parser.cc index 6d24beb..3d2c459 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3,6 +3,18 @@ static Ast wrap_if_several(std::vector &&ast, Ast(*wrapper)(std::vector)); +static inline bool is_identifier_looking(Token::Type type) +{ + switch (type) { + case Token::Type::Symbol: + case Token::Type::Keyword: + case Token::Type::Chord: + return true; + default: + return false; + } +} + constexpr auto Literal_Keywords = std::array { "false"sv, "nil"sv, @@ -39,7 +51,7 @@ Result Parser::parse(std::string_view source, std::string_view filename) auto const result = parser.parse_sequence(); - if (parser.token_id < parser.tokens.size()) { + if (result.has_value() && parser.token_id < parser.tokens.size()) { if (parser.expect(Token::Type::Keyword, "var")) { return Error { .details = errors::Expected_Expression_Separator_Before { .what = "var" }, @@ -71,14 +83,29 @@ Result Parser::parse_variable_declaration() assert(expect(Token::Type::Keyword, "var"), "Parser::parse_variable_declaration must be called only on expressions that starts with 'var'"); auto var = consume(); - auto lvalue = Try(parse_many(*this, &Parser::parse_identifier, std::nullopt, At_Least::One)); + auto lvalue = parse_many(*this, &Parser::parse_identifier, std::nullopt, At_Least::One); + if (not lvalue.has_value()) { + auto details = lvalue.error().details; + if (auto ut = std::get_if(&details); ut) { + return Error { + .details = errors::Literal_As_Identifier { + .type_name = ut->type, + .source = ut->source, + .context = "variable declaration" + }, + .location = lvalue.error().location + }; + } + return std::move(lvalue).error(); + } + if (expect(Token::Type::Operator, "=")) { consume(); - return Ast::variable_declaration(var.location, std::move(lvalue), Try(parse_expression())); + return Ast::variable_declaration(var.location, *std::move(lvalue), Try(parse_expression())); } - return Ast::variable_declaration(var.location, std::move(lvalue), std::nullopt); + return Ast::variable_declaration(var.location, *std::move(lvalue), std::nullopt); } Result Parser::parse_infix_expression() @@ -143,7 +170,35 @@ Result Parser::parse_atomic_expression() return parse_sequence().and_then([&](Ast &&ast) -> Result { if (not expect(Token::Type::Close_Block)) { - unimplemented("Error handling of this code is not implemented yet"); + if (expect(Token::Type::Parameter_Separator)) { + if (is_lambda) { + assert(false, "There should be error message that you cannot put multiple parameter separators in one block"); + } else { + // This may be a result of user trying to specify parameters from things that cannot be parameters (like chord literals) + // or accidential hit of "|" on the keyboard. We can detect first case by ensuring that all tokens between this place + // and beggining of a block are identifier looking: keywords, symbols, literal chord declarations, boolean literals etc + std::optional invalid_token = std::nullopt; + auto const success = std::all_of(tokens.begin() + start, tokens.begin() + (token_id - start + 1), [&](Token const& token) { + if (!invalid_token && token.type != Token::Type::Symbol) { + // TODO Maybe gather all tokens to provide all the help needed in the one iteration + invalid_token = token; + } + return is_identifier_looking(token.type); + }); + + if (success && invalid_token) { + return Error { + .details = errors::Literal_As_Identifier { + .type_name = type_name(invalid_token->type), + .source = invalid_token->source, + .context = "block parameter list" + }, + .location = invalid_token->location + }; + } + } + } + unimplemented("Don't know why it stopped, maybe end of file?"); } consume(); if (is_lambda) { @@ -182,7 +237,7 @@ Result Parser::parse_identifier_with_trailing_separators() // TODO Specific error message return Error { .details = errors::internal::Unexpected_Token { - .type = "", // TODO fill type + .type = type_name(peek()->type), .source = peek()->source, .when = "identifier parsing" }, @@ -200,7 +255,7 @@ Result Parser::parse_identifier() // TODO Specific error message return Error { .details = errors::internal::Unexpected_Token { - .type = "", // TODO fill type + .type = type_name(peek()->type), .source = peek()->source, .when = "identifier parsing" },