diff --git a/Source/Core/Core/DSP/DSPAssembler.cpp b/Source/Core/Core/DSP/DSPAssembler.cpp index 67077f8675..ed181b7a3c 100644 --- a/Source/Core/Core/DSP/DSPAssembler.cpp +++ b/Source/Core/Core/DSP/DSPAssembler.cpp @@ -26,6 +26,42 @@ #include "Core/DSP/DSPDisassembler.h" #include "Core/DSP/DSPTables.h" +template <> +struct fmt::formatter +{ + constexpr auto parse(format_parse_context& ctx) { return ctx.begin(); } + template + auto format(const DSP::DSPAssembler::LocationContext& loc, FormatContext& ctx) const + { + // Trim trailing newlines + // TODO: Ideally, we shouldn't be getting any newlines at all here, but + // DSPTool uses File::ReadFileToString, which opens the file in binary mode, + // so we get \r endings on Windows. This leads to bad results when printing the string. + std::string trimmed_line = loc.line_text; + // while (trimmed_line.ends_with('\r') || trimmed_line.ends_with('\n')) + while (!trimmed_line.empty() && (trimmed_line.back() == '\r' || trimmed_line.back() == '\n')) + trimmed_line.pop_back(); + + auto out = fmt::format_to(ctx.out(), "At line {}", loc.line_num); + + if (loc.included_file_path.has_value()) + out = fmt::format_to(out, " of included file {}", loc.included_file_path.value()); + + if (loc.opcode_type.has_value()) + { + if (loc.opcode_type.value() == DSP::DSPAssembler::OpcodeType::Primary) + out = fmt::format_to(out, ", main opcode"); + else if (loc.opcode_type.value() == DSP::DSPAssembler::OpcodeType::Extension) + out = fmt::format_to(out, ", extension opcode"); + + if (loc.opcode_param_number.has_value()) + out = fmt::format_to(out, " parameter {}", loc.opcode_param_number.value()); + } + + return fmt::format_to(out, ":\n{}", trimmed_line); + } +}; + namespace DSP { static const char* err_string[] = {"", @@ -88,31 +124,39 @@ bool DSPAssembler::Assemble(const std::string& text, std::vector& code, return true; } -void DSPAssembler::ShowError(AssemblerError err_code, const char* extra_info) +void DSPAssembler::ShowError(AssemblerError err_code) { if (!m_settings.force) m_failed = true; - std::string error = fmt::format("{} : {} ", m_code_line, m_cur_line); - if (!extra_info) - extra_info = "-"; - - const char* const error_string = err_string[static_cast(err_code)]; - - if (m_current_param == 0) - { - error += fmt::format("ERROR: {} Line: {} : {}\n", error_string, m_code_line, extra_info); - } - else - { - error += fmt::format("ERROR: {} Line: {} Param: {} : {}\n", error_string, m_code_line, - m_current_param, extra_info); - } - - m_last_error_str = std::move(error); + m_last_error_str = + fmt::format("{}\nERROR: {}\n\n", m_location, err_string[static_cast(err_code)]); + fmt::print(stderr, "{}", m_last_error_str); m_last_error = err_code; } +template +void DSPAssembler::ShowError(AssemblerError err_code, fmt::format_string format, + Args&&... args) +{ + if (!m_settings.force) + m_failed = true; + + const auto msg = fmt::format(format, std::forward(args)...); + + m_last_error_str = fmt::format("{}\nERROR: {}: {}\n\n", m_location, + err_string[static_cast(err_code)], msg); + fmt::print(stderr, "{}", m_last_error_str); + m_last_error = err_code; +} + +template +void DSPAssembler::ShowWarning(fmt::format_string format, Args&&... args) +{ + const auto msg = fmt::format(format, std::forward(args)...); + fmt::print(stderr, "{}\nWARNING: {}\n\n", m_location, msg); +} + static char* skip_spaces(char* ptr) { while (*ptr == ' ') @@ -147,7 +191,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '9') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectDecimal, str); + ShowError(AssemblerError::IncorrectDecimal, "{}", str); } } else @@ -165,7 +209,7 @@ s32 DSPAssembler::ParseValue(const char* str) else if (ptr[i] >= '0' && ptr[i] <= '9') val += (ptr[i] - '0'); else - ShowError(AssemblerError::IncorrectHex, str); + ShowError(AssemblerError::IncorrectHex, "{}", str); } break; case '\'': // binary @@ -175,7 +219,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '1') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectBinary, str); + ShowError(AssemblerError::IncorrectBinary, "{}", str); } break; default: @@ -196,7 +240,7 @@ s32 DSPAssembler::ParseValue(const char* str) if (ptr[i] >= '0' && ptr[i] <= '9') val += ptr[i] - '0'; else - ShowError(AssemblerError::IncorrectDecimal, str); + ShowError(AssemblerError::IncorrectDecimal, "{}", str); } } else // Everything else is a label. @@ -206,7 +250,7 @@ s32 DSPAssembler::ParseValue(const char* str) return *value; if (m_cur_pass == 2) - ShowError(AssemblerError::UnknownLabel, str); + ShowError(AssemblerError::UnknownLabel, "{}", str); } } if (negative) @@ -332,10 +376,10 @@ u32 DSPAssembler::ParseExpression(const char* ptr) val = ParseExpression(d_buffer) - ParseExpression(pbuf + 1); if (val < 0) { + ShowWarning("Number Underflow: {}", val); val = 0x10000 + (val & 0xffff); // ATTENTION: avoid a terrible bug!!! number cannot write with '-' in sprintf - fprintf(stderr, "WARNING: Number Underflow at Line: %d \n", m_code_line); } sprintf(d_buffer, "%d", val); } @@ -472,7 +516,7 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t { for (size_t i = 0; i < count; i++) { - const size_t current_param = i + 1; // just for display. + m_location.opcode_param_number = i + 1; if (opc->params[i].type != par[i].type || (par[i].type & P_REG)) { if (par[i].type == P_VAL && @@ -496,10 +540,6 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t if ((int)par[i].val < value || (int)par[i].val > value + get_mask_shifted_down(opc->params[i].mask)) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - - fprintf(stderr, "%s (param %zu)", m_cur_line.c_str(), current_param); ShowError(AssemblerError::InvalidRegister); } break; @@ -507,34 +547,19 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_PRG: if ((int)par[i].val < DSP_REG_AR0 || (int)par[i].val > DSP_REG_AR3) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - - fprintf(stderr, "%s (param %zu)", m_cur_line.c_str(), current_param); ShowError(AssemblerError::InvalidRegister); } break; case P_ACC: if ((int)par[i].val < DSP_REG_ACC0_FULL || (int)par[i].val > DSP_REG_ACC1_FULL) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { - fprintf(stderr, "%i : %s ", m_code_line, m_cur_line.c_str()); - fprintf(stderr, - "WARNING: $ACM%d register used instead of $ACC%d register Line: %d " - "Param: %zu Ext: %d\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param, - static_cast(type)); + ShowWarning("$ACM{0} register used instead of $ACC{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { - fprintf( - stderr, - "WARNING: $ACL%d register used instead of $ACC%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACL{0} register used instead of $ACC{0} register", (par[i].val & 1)); } else { @@ -545,22 +570,13 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_ACCM: if ((int)par[i].val < DSP_REG_ACM0 || (int)par[i].val > DSP_REG_ACM1) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACL0 && par[i].val <= DSP_REG_ACL1) { - fprintf( - stderr, - "WARNING: $ACL%d register used instead of $ACM%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACL{0} register used instead of $ACCM{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { - fprintf( - stderr, - "WARNING: $ACC%d register used instead of $ACM%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACC{0} register used instead of $ACM{0} register", (par[i].val & 1)); } else { @@ -572,24 +588,13 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t case P_ACCL: if ((int)par[i].val < DSP_REG_ACL0 || (int)par[i].val > DSP_REG_ACL1) { - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); - if (par[i].val >= DSP_REG_ACM0 && par[i].val <= DSP_REG_ACM1) { - fprintf(stderr, "%s ", m_cur_line.c_str()); - fprintf( - stderr, - "WARNING: $ACM%d register used instead of $ACL%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACM{0} register used instead of $ACL{0} register", (par[i].val & 1)); } else if (par[i].val >= DSP_REG_ACC0_FULL && par[i].val <= DSP_REG_ACC1_FULL) { - fprintf(stderr, "%s ", m_cur_line.c_str()); - fprintf( - stderr, - "WARNING: $ACC%d register used instead of $ACL%d register Line: %d Param: %zu\n", - (par[i].val & 1), (par[i].val & 1), m_code_line, current_param); + ShowWarning("$ACC{0} register used instead of $ACL{0} register", (par[i].val & 1)); } else { @@ -604,23 +609,15 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t switch (par[i].type & (P_REG | 7)) { case P_REG: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamReg); break; case P_MEM: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamMem); break; case P_VAL: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamVal); break; case P_IMM: - if (type == OpcodeType::Extension) - fprintf(stderr, "(ext) "); ShowError(AssemblerError::ExpectedParamImm); break; } @@ -634,40 +631,49 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t unsigned int valueu = 0xffff & ~(value >> 1); if ((int)par[i].val < 0) { - if (value == 7) // value 7 por sbclr/sbset + if (value == 7) // value 7 for sbclr/sbset { - fprintf(stderr, "Value must be from 0x0 to 0x%x\n", value); - ShowError(AssemblerError::NumberOutOfRange); + ShowError(AssemblerError::NumberOutOfRange, "Value must be from 0x0 to {:#x}, was {:#x}", + value, (int)par[i].val); } else if (opc->params[i].type == P_MEM) { if (value < 256) - fprintf(stderr, "Address value must be from 0x%x to 0x%x\n", valueu, (value >> 1)); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from {:#x} to {:#x}, was {:#x}", valueu, (value >> 1), + (int)par[i].val); + } else - fprintf(stderr, "Address value must be from 0x0 to 0x%x\n", value); - - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from 0x0 to {:#x}, was {:#x}", value, (int)par[i].val); + } } else if ((int)par[i].val < -((value >> 1) + 1)) { if (value < 128) - fprintf(stderr, "Value must be from -0x%x to 0x%x, is %i\n", (value >> 1) + 1, - value >> 1, par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}", -((value >> 1) + 1), + value >> 1, (int)par[i].val); + } else - fprintf(stderr, "Value must be from -0x%x to 0x%x or 0x0 to 0x%x, is %i\n", - (value >> 1) + 1, value >> 1, value, par[i].val); - - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x} or 0x0 to {:#x}, was {:#x}", + -((value >> 1) + 1), value >> 1, value, (int)par[i].val); + } } } else { - if (value == 7) // value 7 por sbclr/sbset + if (value == 7) // value 7 for sbclr/sbset { if (par[i].val > (unsigned)value) { - fprintf(stderr, "Value must be from 0x%x to 0x%x, is %i\n", valueu, value, par[i].val); - ShowError(AssemblerError::NumberOutOfRange); + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}\n", valueu, value, par[i].val); } } else if (opc->params[i].type == P_MEM) @@ -678,11 +684,17 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t (par[i].val < valueu || par[i].val > (unsigned)0xffff)) { if (value < 256) - fprintf(stderr, "Address value must be from 0x%x to 0x%x, is %04x\n", valueu, value, - par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be from {:#x} to {:#x}, was {:04x}\n", valueu, value, + par[i].val); + } else - fprintf(stderr, "Address value must be minor of 0x%x\n", value + 1); - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Address value must be less than {:#x}, was {:04x}\n", value + 1, + par[i].val); + } } } else @@ -692,18 +704,23 @@ bool DSPAssembler::VerifyParams(const DSPOPCTemplate* opc, param_t* par, size_t if (par[i].val > (unsigned)value) { if (value < 64) - fprintf(stderr, "Value must be from -0x%x to 0x%x, is %i\n", (value + 1), value, - par[i].val); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be from {:#x} to {:#x}, was {:#x}\n", -(value + 1), value, + par[i].val); + } else - fprintf(stderr, "Value must be minor of 0x%x, is %i\n", value + 1, par[i].val); - ShowError(AssemblerError::NumberOutOfRange); + { + ShowError(AssemblerError::NumberOutOfRange, + "Value must be less than {:#x}, was {:#x}\n", value + 1, par[i].val); + } } } } continue; } } - m_current_param = 0; + m_location.opcode_param_number = std::nullopt; return true; } @@ -755,7 +772,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) std::istringstream fsrc(text); - m_code_line = 0; + m_location.line_num = 0; m_cur_pass = pass; #define LINEBUF_SIZE 1024 @@ -767,8 +784,8 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) if (fsrc.fail()) break; - m_cur_line = line; - m_code_line++; + m_location.line_text = line; + m_location.line_num++; param_t params[10] = {{0, P_NONE, nullptr}}; param_t params_ext[10] = {{0, P_NONE, nullptr}}; @@ -905,23 +922,22 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) { if (params[0].type == P_STR) { - std::string include_file_path; - const u32 this_code_line = m_code_line; + const auto old_location = m_location; if (m_include_dir.empty()) { - include_file_path = params[0].str; + m_location.included_file_path = params[0].str; } else { - include_file_path = m_include_dir + '/' + params[0].str; + m_location.included_file_path = m_include_dir + '/' + params[0].str; } std::string included_text; - File::ReadFileToString(include_file_path, included_text); + File::ReadFileToString(m_location.included_file_path.value(), included_text); AssemblePass(included_text, pass); - m_code_line = this_code_line; + m_location = old_location; } else { @@ -959,9 +975,8 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) { if (m_cur_addr > params[0].val) { - const std::string msg = fmt::format("WARNPC at 0x{:04x}, expected 0x{:04x} or less", - m_cur_addr, params[0].val); - ShowError(AssemblerError::PCOutOfRange, msg.c_str()); + ShowError(AssemblerError::PCOutOfRange, "WARNPC at 0x{:04x}, expected 0x{:04x} or less", + m_cur_addr, params[0].val); } } else @@ -987,6 +1002,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) continue; } + m_location.opcode_type = OpcodeType::Primary; const DSPOPCTemplate* opc = FindOpcode(opcode, params_count, OpcodeType::Primary); if (!opc) opc = &cw; @@ -997,6 +1013,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) const DSPOPCTemplate* opc_ext = nullptr; // Check for opcode extensions. + m_location.opcode_type = OpcodeType::Extension; if (opc->extended) { if (opcode_ext) @@ -1016,6 +1033,7 @@ bool DSPAssembler::AssemblePass(const std::string& text, int pass) if (params_count_ext) ShowError(AssemblerError::ExtensionParamsOnNonExtendableOpcode); } + m_location.opcode_type = std::nullopt; if (pass == 2) { diff --git a/Source/Core/Core/DSP/DSPAssembler.h b/Source/Core/Core/DSP/DSPAssembler.h index 4c1cdbcd94..29ccaeb557 100644 --- a/Source/Core/Core/DSP/DSPAssembler.h +++ b/Source/Core/Core/DSP/DSPAssembler.h @@ -6,9 +6,13 @@ #include #include +#include #include +#include #include +#include + #include "Common/CommonTypes.h" #include "Core/DSP/DSPDisassembler.h" @@ -85,6 +89,17 @@ private: Extension }; + struct LocationContext + { + u32 line_num = 0; + std::string line_text; + std::optional opcode_type; + std::optional opcode_param_number; + std::optional included_file_path; + }; + + friend struct ::fmt::formatter; + // Utility functions s32 ParseValue(const char* str); u32 ParseExpression(const char* ptr); @@ -94,7 +109,11 @@ private: void InitPass(int pass); bool AssemblePass(const std::string& text, int pass); - void ShowError(AssemblerError err_code, const char* extra_info = nullptr); + void ShowError(AssemblerError err_code); + template + void ShowError(AssemblerError err_code, fmt::format_string format, Args&&... args); + template + void ShowWarning(fmt::format_string format, Args&&... args); char* FindBrackets(char* src, char* dst); const DSPOPCTemplate* FindOpcode(std::string name, size_t par_count, OpcodeType type); @@ -104,7 +123,6 @@ private: std::vector m_output_buffer; std::string m_include_dir; - std::string m_cur_line; u32 m_cur_addr = 0; int m_total_size = 0; @@ -112,7 +130,6 @@ private: LabelMap m_labels; - u32 m_code_line = 0; bool m_failed = false; std::string m_last_error_str; AssemblerError m_last_error = AssemblerError::OK; @@ -122,7 +139,8 @@ private: segment_t m_cur_segment = SEGMENT_CODE; u32 m_segment_addr[SEGMENT_MAX] = {}; - int m_current_param = 0; const AssemblerSettings m_settings; + + LocationContext m_location; }; } // namespace DSP diff --git a/Source/Core/Core/DSP/DSPCodeUtil.cpp b/Source/Core/Core/DSP/DSPCodeUtil.cpp index 98e8c44d59..9a33edc9ba 100644 --- a/Source/Core/Core/DSP/DSPCodeUtil.cpp +++ b/Source/Core/Core/DSP/DSPCodeUtil.cpp @@ -35,13 +35,7 @@ bool Assemble(const std::string& text, std::vector& code, bool force) // TODO: fix the terrible api of the assembler. DSPAssembler assembler(settings); - if (!assembler.Assemble(text, code)) - { - std::cerr << assembler.GetErrorString() << std::endl; - return false; - } - - return true; + return assembler.Assemble(text, code); } bool Disassemble(const std::vector& code, bool line_numbers, std::string& text)