From 881daae15c800302ab00c8563ecdd048c7059445 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Thu, 17 Oct 2024 08:02:48 -0300 Subject: [PATCH 1/6] #983: Refactor CTPG script options Java parser code --- .../parser_ctpg_script_importer.cc | 50 +++++++++---------- .../parser_ctpg_script_importer.h | 17 ++++++- .../ctpg/script_option_lines_ctpg.cc | 50 ++++++++++++++----- 3 files changed, 78 insertions(+), 39 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 226e2359..418de5cd 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -27,6 +27,30 @@ void ScriptImporter::importScript(std::string & scriptCode, importScript(scriptCode, options, 0); } +void ScriptImporter::replaceScripts(const ExecutionGraph::OptionsLineParser::CTPG::options_map_t::mapped_type & option_values, + const size_t recursionDepth, + std::vector &result) { + for (const auto & option: option_values) { + const char *importScriptCode = findImportScript(option.value); + std::string importScriptCodeStr; + if (m_importedScriptChecksums.addScript(importScriptCode) ) { + // Script has not been imported yet + // If this imported script contains %import statements + // they will be resolved in the next recursion. + ctpg_parser::options_map_t newOptions; + try { + ExecutionGraph::OptionsLineParser::CTPG::parseOptions(importScriptCode, newOptions); + } catch(const ExecutionGraph::OptionParserException & ex) { + Utils::rethrow(ex, "F-UDF-CL-SL-JAVA-1630"); + } + importScriptCodeStr.assign(importScriptCode); + importScript(importScriptCodeStr, newOptions, recursionDepth + 1); + } + ReplacedScripts replacedScript = {.script = std::move(importScriptCodeStr), .origPos = option.idx_in_source, .origLen = option.size }; + result.push_back(std::move(replacedScript)); + } +} + void ScriptImporter::importScript(std::string & scriptCode, ctpg_parser::options_map_t & options, const size_t recursionDepth) { @@ -43,35 +67,11 @@ void ScriptImporter::importScript(std::string & scriptCode, { return first.idx_in_source < second.idx_in_source; }); - struct ReplacedScripts { - ReplacedScripts(ReplacedScripts&&) = default; - std::string script; - size_t origPos; - size_t origLen; - }; std::vector replacedScripts; replacedScripts.reserve(optionIt->second.size()); //In order to continue compatibility with legacy implementation we must collect import scripts in forward direction //but then replace in reverse direction (in order to keep consistency of positions) - for (const auto & option: optionIt->second) { - const char *importScriptCode = findImportScript(option.value); - std::string importScriptCodeStr; - if (m_importedScriptChecksums.addScript(importScriptCode) ) { - // Script has not been imported yet - // If this imported script contains %import statements - // they will be resolved in the next recursion. - ctpg_parser::options_map_t newOptions; - try { - ExecutionGraph::OptionsLineParser::CTPG::parseOptions(importScriptCode, newOptions); - } catch(const ExecutionGraph::OptionParserException & ex) { - Utils::rethrow(ex, "F-UDF-CL-SL-JAVA-1630"); - } - importScriptCodeStr.assign(importScriptCode); - importScript(importScriptCodeStr, newOptions, recursionDepth + 1); - } - ReplacedScripts replacedScript = {.script = std::move(importScriptCodeStr), .origPos = option.idx_in_source, .origLen = option.size }; - replacedScripts.push_back(std::move(replacedScript)); - } + replaceScripts(optionIt->second, recursionDepth, replacedScripts); //Now replace the imported script bodies from end to start. Doing it in forward order would invalidate the offsets of later import scripts. for (auto optionIt = replacedScripts.rbegin(); optionIt != replacedScripts.rend(); optionIt++) { scriptCode.replace(optionIt->origPos, optionIt->origLen, optionIt->script); diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h index f0492e34..07624254 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h @@ -16,6 +16,8 @@ namespace JavaScriptOptions { namespace CTPG { + + class ScriptImporter { public: @@ -23,17 +25,30 @@ class ScriptImporter { void importScript(std::string & scriptCode, ExecutionGraph::OptionsLineParser::CTPG::options_map_t & options); + private: + struct ReplacedScripts { + ReplacedScripts(ReplacedScripts&&) = default; + std::string script; + size_t origPos; + size_t origLen; + }; + private: void importScript(std::string & scriptCode, ExecutionGraph::OptionsLineParser::CTPG::options_map_t & options, const size_t recursionDepth); const char* findImportScript(const std::string & scriptKey); + + void replaceScripts(const ExecutionGraph::OptionsLineParser::CTPG::options_map_t::mapped_type & option_values, + const size_t recursionDepth, + std::vector &result); + private: Checksum m_importedScriptChecksums; SwigFactory & m_swigFactory; std::unique_ptr m_metaData; Keywords & m_keywords; - //The empirical maximal value for recursion depth is ~26000. So we choose 20000 to have a certain buffer. + //The empirical maximal value for recursion depth is ~18000. So we choose 10000 to have a certain buffer. const size_t cMaxRecursionDepth = 20000; }; diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc index 74d799de..e89a948b 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc @@ -200,29 +200,53 @@ void parse(std::string&& code, options_type& result) { } //namespace ParserInternals +struct LinePositions { + size_t mStartPos; + size_t mEndPos; +}; + +inline std::optional getNextLine(const size_t current_pos, const std::string & scriptCode) { + /** + * Find first of occurence of '%', starting search from position 'current_pos'. + * If no '%' is found, return an empty result. + * If '%' is found, search backwards from '%' for '\n' or \r': + * 1. If not found, '%' was found in the first line. Then we can set 'new_option_start_pos'=0 + * 2. If found, set new_option_start_pos to position 1 char behind pos of found '\n' or '\r'. + * Then search forward for next occurence of '\n' or \r' and assign to var 'line_end_pos': + 1. If not found, 'line_end_pos' will get assigned std::string::npos (std::string::substr(...,npos), returns substring until end of string + 2. If found, 'line_end_pos' will assigned to position of line end of line where '%' was found + */ + std::optional retVal; + const size_t new_option_start_pos = scriptCode.find_first_of("%", current_pos); + if (new_option_start_pos == std::string::npos) + return retVal; + size_t line_start_pos = scriptCode.find_last_of("\r\n", new_option_start_pos); + if (std::string::npos == line_start_pos) + line_start_pos = 0; + else + line_start_pos++; + + const size_t line_end_pos = scriptCode.find_first_of("\r\n", line_start_pos); + retVal = LinePositions{ .mStartPos = line_start_pos, .mEndPos = line_end_pos}; + return retVal; +} + void parseOptions(const std::string& code, options_map_t & result) { size_t current_pos = 0; do { - const size_t new_option_start_pos = code.find_first_of("%", current_pos); - if (new_option_start_pos == std::string::npos) + const std::optional currentLinePositions = getNextLine(current_pos, code); + if (!currentLinePositions) break; - current_pos = code.find_last_of("\r\n", new_option_start_pos); - if (std::string::npos == current_pos) - current_pos = 0; - else - current_pos++; - - const size_t new_pos = code.find_first_of("\r\n", current_pos); - std::string line = code.substr(current_pos, new_pos); + std::string line = code.substr(currentLinePositions->mStartPos, currentLinePositions->mEndPos); options_type parser_result; ParserInternals::parse(std::move(line), parser_result); for (const auto & option: parser_result) { ScriptOption entry = { .value = option.value, - .idx_in_source = current_pos + option.start.column - 1, + .idx_in_source = currentLinePositions->mStartPos + option.start.column - 1, .size = option.end.column - option.start.column + 1 }; auto it_in_result = result.find(option.key); @@ -237,10 +261,10 @@ void parseOptions(const std::string& code, options_map_t & result) { it_in_result->second.push_back(entry); } } - if (new_pos == std::string::npos) { + if (currentLinePositions->mEndPos == std::string::npos) { break; } - current_pos = new_pos + 1; + current_pos = currentLinePositions->mEndPos + 1; } while(true); } From c53d4b2b640024d3b2c0d7d167651a047fff2d69 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Fri, 18 Oct 2024 07:15:24 -0300 Subject: [PATCH 2/6] Fixes from review --- .../parser_ctpg_script_importer.cc | 8 +++-- .../parser_ctpg_script_importer.h | 10 +++--- .../ctpg/script_option_lines_ctpg.cc | 36 +++++++++---------- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 418de5cd..4f41ae25 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -27,7 +27,7 @@ void ScriptImporter::importScript(std::string & scriptCode, importScript(scriptCode, options, 0); } -void ScriptImporter::replaceScripts(const ExecutionGraph::OptionsLineParser::CTPG::options_map_t::mapped_type & option_values, +void ScriptImporter::replaceScripts(const ScriptImporter::OptionValues_t & option_values, const size_t recursionDepth, std::vector &result) { for (const auto & option: option_values) { @@ -82,13 +82,15 @@ void ScriptImporter::importScript(std::string & scriptCode, const char* ScriptImporter::findImportScript(const std::string & scriptKey) { if (!m_metaData) { m_metaData.reset(m_swigFactory.makeSwigMetadata()); - if (!m_metaData) + if (!m_metaData) { throw std::runtime_error("F-UDF-CL-SL-JAVA-1631: Failure while importing scripts"); + } } const char *importScriptCode = m_metaData->moduleContent(scriptKey.c_str()); const char *exception = m_metaData->checkException(); - if (exception) + if (exception) { throw std::runtime_error("F-UDF-CL-SL-JAVA-1632: " + std::string(exception)); + } return importScriptCode; } diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h index 07624254..be9b7792 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h @@ -33,23 +33,23 @@ class ScriptImporter { size_t origLen; }; - private: + typedef ExecutionGraph::OptionsLineParser::CTPG::options_map_t::mapped_type OptionValues_t; + void importScript(std::string & scriptCode, ExecutionGraph::OptionsLineParser::CTPG::options_map_t & options, const size_t recursionDepth); const char* findImportScript(const std::string & scriptKey); - void replaceScripts(const ExecutionGraph::OptionsLineParser::CTPG::options_map_t::mapped_type & option_values, + void replaceScripts(const OptionValues_t & option_values, const size_t recursionDepth, std::vector &result); - private: Checksum m_importedScriptChecksums; SwigFactory & m_swigFactory; std::unique_ptr m_metaData; Keywords & m_keywords; - //The empirical maximal value for recursion depth is ~18000. So we choose 10000 to have a certain buffer. - const size_t cMaxRecursionDepth = 20000; + //The empirical maximal value for recursion depth is ~18000. So we add a little bit extra to have some buffer. + const size_t cMaxRecursionDepth = 10000U; }; } //namespace CTPG diff --git a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc index e89a948b..ad6501c8 100644 --- a/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc +++ b/exaudfclient/base/script_options_parser/ctpg/script_option_lines_ctpg.cc @@ -186,12 +186,10 @@ void parse(std::string&& code, options_type& result) { parse_options{}.set_skip_whitespace(false), buffers::string_buffer(std::move(code)), error_buffer); - if (res.has_value()) - { + if (res.has_value()) { result = res.value(); } - else - { + else { std::stringstream ss; ss << "Error parsing script options: " << error_buffer.str(); throw OptionParserException(ss.str()); @@ -205,7 +203,7 @@ struct LinePositions { size_t mEndPos; }; -inline std::optional getNextLine(const size_t current_pos, const std::string & scriptCode) { +inline std::optional getNextLine(const size_t current_pos, const std::string & scriptCode) { /** * Find first of occurence of '%', starting search from position 'current_pos'. * If no '%' is found, return an empty result. @@ -218,13 +216,16 @@ inline std::optional getNextLine(const size_t current_pos, */ std::optional retVal; const size_t new_option_start_pos = scriptCode.find_first_of("%", current_pos); - if (new_option_start_pos == std::string::npos) + if (new_option_start_pos == std::string::npos) { return retVal; + } size_t line_start_pos = scriptCode.find_last_of("\r\n", new_option_start_pos); - if (std::string::npos == line_start_pos) + if (std::string::npos == line_start_pos) { line_start_pos = 0; - else + } + else { line_start_pos++; + } const size_t line_end_pos = scriptCode.find_first_of("\r\n", line_start_pos); retVal = LinePositions{ .mStartPos = line_start_pos, .mEndPos = line_end_pos}; @@ -234,30 +235,25 @@ inline std::optional getNextLine(const size_t current_pos, void parseOptions(const std::string& code, options_map_t & result) { size_t current_pos = 0; + std::optional currentLinePositions = getNextLine(current_pos, code); + while (currentLinePositions) { - do { - const std::optional currentLinePositions = getNextLine(current_pos, code); - if (!currentLinePositions) - break; std::string line = code.substr(currentLinePositions->mStartPos, currentLinePositions->mEndPos); options_type parser_result; ParserInternals::parse(std::move(line), parser_result); - for (const auto & option: parser_result) - { + for (const auto & option: parser_result) { ScriptOption entry = { .value = option.value, .idx_in_source = currentLinePositions->mStartPos + option.start.column - 1, .size = option.end.column - option.start.column + 1 }; auto it_in_result = result.find(option.key); - if (it_in_result == result.end()) - { + if (it_in_result == result.end()) { options_t new_options; new_options.push_back(entry); result.insert(std::make_pair(option.key, new_options)); } - else - { + else { it_in_result->second.push_back(entry); } } @@ -265,7 +261,9 @@ void parseOptions(const std::string& code, options_map_t & result) { break; } current_pos = currentLinePositions->mEndPos + 1; - } while(true); + + currentLinePositions = getNextLine(current_pos, code); + } } From 4382eb7a212037998da09653d45e53d9cc31ce8f Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:24:54 -0300 Subject: [PATCH 3/6] Fixes from review --- .../script_options/parser_ctpg_script_importer.cc | 8 ++++---- .../script_options/parser_ctpg_script_importer.h | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 4f41ae25..10d9f0d6 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -27,9 +27,9 @@ void ScriptImporter::importScript(std::string & scriptCode, importScript(scriptCode, options, 0); } -void ScriptImporter::replaceScripts(const ScriptImporter::OptionValues_t & option_values, - const size_t recursionDepth, - std::vector &result) { +void ScriptImporter::collectImportScripts(const ScriptImporter::OptionValues_t & option_values, + const size_t recursionDepth, + std::vector &result) { for (const auto & option: option_values) { const char *importScriptCode = findImportScript(option.value); std::string importScriptCodeStr; @@ -71,7 +71,7 @@ void ScriptImporter::importScript(std::string & scriptCode, replacedScripts.reserve(optionIt->second.size()); //In order to continue compatibility with legacy implementation we must collect import scripts in forward direction //but then replace in reverse direction (in order to keep consistency of positions) - replaceScripts(optionIt->second, recursionDepth, replacedScripts); + collectImportScripts(optionIt->second, recursionDepth, replacedScripts); //Now replace the imported script bodies from end to start. Doing it in forward order would invalidate the offsets of later import scripts. for (auto optionIt = replacedScripts.rbegin(); optionIt != replacedScripts.rend(); optionIt++) { scriptCode.replace(optionIt->origPos, optionIt->origLen, optionIt->script); diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h index be9b7792..644d8fc5 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h @@ -40,9 +40,9 @@ class ScriptImporter { const size_t recursionDepth); const char* findImportScript(const std::string & scriptKey); - void replaceScripts(const OptionValues_t & option_values, - const size_t recursionDepth, - std::vector &result); + void collectImportScripts(const OptionValues_t & option_values, + const size_t recursionDepth, + std::vector &result); Checksum m_importedScriptChecksums; SwigFactory & m_swigFactory; From 05d882442e435f6661e3136ad2b774224e772ba9 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:45:56 -0300 Subject: [PATCH 4/6] Renamed ScriptImporter::ReplacedScripts => ScriptImporter::CollectedScript --- .../script_options/parser_ctpg_script_importer.cc | 12 ++++++------ .../script_options/parser_ctpg_script_importer.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 10d9f0d6..c516b48f 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -29,7 +29,7 @@ void ScriptImporter::importScript(std::string & scriptCode, void ScriptImporter::collectImportScripts(const ScriptImporter::OptionValues_t & option_values, const size_t recursionDepth, - std::vector &result) { + std::vector &result) { for (const auto & option: option_values) { const char *importScriptCode = findImportScript(option.value); std::string importScriptCodeStr; @@ -46,7 +46,7 @@ void ScriptImporter::collectImportScripts(const ScriptImporter::OptionValues_t & importScriptCodeStr.assign(importScriptCode); importScript(importScriptCodeStr, newOptions, recursionDepth + 1); } - ReplacedScripts replacedScript = {.script = std::move(importScriptCodeStr), .origPos = option.idx_in_source, .origLen = option.size }; + CollectedScript replacedScript = {.script = std::move(importScriptCodeStr), .origPos = option.idx_in_source, .origLen = option.size }; result.push_back(std::move(replacedScript)); } } @@ -67,13 +67,13 @@ void ScriptImporter::importScript(std::string & scriptCode, { return first.idx_in_source < second.idx_in_source; }); - std::vector replacedScripts; - replacedScripts.reserve(optionIt->second.size()); + std::vector CollectedScript; + CollectedScript.reserve(optionIt->second.size()); //In order to continue compatibility with legacy implementation we must collect import scripts in forward direction //but then replace in reverse direction (in order to keep consistency of positions) - collectImportScripts(optionIt->second, recursionDepth, replacedScripts); + collectImportScripts(optionIt->second, recursionDepth, CollectedScript); //Now replace the imported script bodies from end to start. Doing it in forward order would invalidate the offsets of later import scripts. - for (auto optionIt = replacedScripts.rbegin(); optionIt != replacedScripts.rend(); optionIt++) { + for (auto optionIt = CollectedScript.rbegin(); optionIt != CollectedScript.rend(); optionIt++) { scriptCode.replace(optionIt->origPos, optionIt->origLen, optionIt->script); } } diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h index 644d8fc5..07aca4f8 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h @@ -26,8 +26,8 @@ class ScriptImporter { void importScript(std::string & scriptCode, ExecutionGraph::OptionsLineParser::CTPG::options_map_t & options); private: - struct ReplacedScripts { - ReplacedScripts(ReplacedScripts&&) = default; + struct CollectedScript { + CollectedScript(CollectedScript&&) = default; std::string script; size_t origPos; size_t origLen; @@ -42,7 +42,7 @@ class ScriptImporter { void collectImportScripts(const OptionValues_t & option_values, const size_t recursionDepth, - std::vector &result); + std::vector &result); Checksum m_importedScriptChecksums; SwigFactory & m_swigFactory; From b3990cc676fd998f25d038f79b6891b5690a0c0e Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:51:59 -0300 Subject: [PATCH 5/6] Extracted script replacement into separate function --- .../parser_ctpg_script_importer.cc | 22 +++++++++++++------ .../parser_ctpg_script_importer.h | 3 +++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index c516b48f..4cf9860a 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -51,6 +51,16 @@ void ScriptImporter::collectImportScripts(const ScriptImporter::OptionValues_t & } } +void ScriptImporter::replaceImportScripts(std::string & scriptCode, + const std::vector &collectedImportScripts) { + //Replace the imported script bodies from end to start. + //Doing it in forward order would invalidate the offsets of later import scripts. + for (auto optionIt = collectedImportScripts.crbegin(); optionIt != collectedImportScripts.crend(); optionIt++) { + scriptCode.replace(optionIt->origPos, optionIt->origLen, optionIt->script); + } +} + + void ScriptImporter::importScript(std::string & scriptCode, ctpg_parser::options_map_t & options, const size_t recursionDepth) { @@ -67,15 +77,13 @@ void ScriptImporter::importScript(std::string & scriptCode, { return first.idx_in_source < second.idx_in_source; }); - std::vector CollectedScript; - CollectedScript.reserve(optionIt->second.size()); + std::vector collectedScript; + collectedScript.reserve(optionIt->second.size()); //In order to continue compatibility with legacy implementation we must collect import scripts in forward direction //but then replace in reverse direction (in order to keep consistency of positions) - collectImportScripts(optionIt->second, recursionDepth, CollectedScript); - //Now replace the imported script bodies from end to start. Doing it in forward order would invalidate the offsets of later import scripts. - for (auto optionIt = CollectedScript.rbegin(); optionIt != CollectedScript.rend(); optionIt++) { - scriptCode.replace(optionIt->origPos, optionIt->origLen, optionIt->script); - } + collectImportScripts(optionIt->second, recursionDepth, collectedScript); + //Now replace the imported script bodies + replaceImportScripts(scriptCode, collectedScript); } } diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h index 07aca4f8..33b14d55 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.h @@ -44,6 +44,9 @@ class ScriptImporter { const size_t recursionDepth, std::vector &result); + void replaceImportScripts(std::string & scriptCode, + const std::vector &collectedImportScripts); + Checksum m_importedScriptChecksums; SwigFactory & m_swigFactory; std::unique_ptr m_metaData; From 0fb3e07a0c950fb110c829382f684bd42cad26a4 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:25:12 -0300 Subject: [PATCH 6/6] Fixes from review --- .../script_options/parser_ctpg_script_importer.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc index 4cf9860a..84dfb708 100644 --- a/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc +++ b/exaudfclient/base/javacontainer/script_options/parser_ctpg_script_importer.cc @@ -77,13 +77,12 @@ void ScriptImporter::importScript(std::string & scriptCode, { return first.idx_in_source < second.idx_in_source; }); - std::vector collectedScript; - collectedScript.reserve(optionIt->second.size()); - //In order to continue compatibility with legacy implementation we must collect import scripts in forward direction - //but then replace in reverse direction (in order to keep consistency of positions) - collectImportScripts(optionIt->second, recursionDepth, collectedScript); - //Now replace the imported script bodies - replaceImportScripts(scriptCode, collectedScript); + std::vector collectedScripts; + collectedScripts.reserve(optionIt->second.size()); + //In order to continue compatibility with legacy implementation + //we must collect import scripts in forward direction but then replace in reverse direction. + collectImportScripts(optionIt->second, recursionDepth, collectedScripts); + replaceImportScripts(scriptCode, collectedScripts); } }