From def2171378068baecc4c690d0f8dbdfec4ae6bac Mon Sep 17 00:00:00 2001 From: "Peter G Bouillon [fernewelten]" Date: Wed, 11 Dec 2024 23:10:35 +0100 Subject: [PATCH 1/5] Editor: Preprocessor honors new script markers Also, when the preprocessor finds unterminated strings, it tells what character is missing (quote or double-quote sign) --- Editor/AGS.CScript.Compiler/Preprocessor.cs | 32 +++++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/Editor/AGS.CScript.Compiler/Preprocessor.cs b/Editor/AGS.CScript.Compiler/Preprocessor.cs index 0c2fe4cdd12..3a7007297a2 100644 --- a/Editor/AGS.CScript.Compiler/Preprocessor.cs +++ b/Editor/AGS.CScript.Compiler/Preprocessor.cs @@ -99,14 +99,26 @@ private string PreProcessLine(string lineToProcess) int i = 0; while ((i < line.Length) && (!Char.IsLetterOrDigit(line[i]))) { - if ((line[i] == '"') || (line[i] == '\'')) - { - i = FindIndexOfMatchingCharacter(line.ToString(), i, line[i]); - if (i < 0) - { - i = line.Length; - break; - } + if ((line[i] == '"') || (line[i] == '\'')) + { + int end_of_literal = FindIndexOfMatchingCharacter(line.ToString(), i, line[i]); + if (end_of_literal < 0) + { + i = line.Length; + break; + } + if (i == 0 && line[0] == '"') + { + // '[end_of_literal]' contains the '"', we need the part before that + FastString literal = line.Substring(0, end_of_literal); + if (literal.StartsWith(Constants.NEW_SCRIPT_MARKER)) + { + // Start the new script + _scriptName = literal.Substring(Constants.NEW_SCRIPT_MARKER.Length).ToString(); + _lineNumber = 0; + } + } + i = end_of_literal; } i++; } @@ -388,7 +400,9 @@ private string RemoveComments(string text) int endOfString = FindIndexOfMatchingCharacter(text, i, text[i]); if (endOfString < 0) { - RecordError(ErrorCode.UnterminatedString, "Unterminated string"); + RecordError( + ErrorCode.UnterminatedString, + $"Unterminated string: {text[i]} is missing"); break; } endOfString++; From 1806ee2a82853967868f28ffeb203c28cecee162 Mon Sep 17 00:00:00 2001 From: "Peter G Bouillon [fernewelten]" Date: Wed, 11 Dec 2024 23:54:52 +0100 Subject: [PATCH 2/5] Classic compiler: Preprocessor honors new script markers Also, when the preprocessor finds unterminated strings, it tells what character is missing (quote or double-quote sign) --- Compiler/preproc/preprocessor.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/Compiler/preproc/preprocessor.cpp b/Compiler/preproc/preprocessor.cpp index 8046e5fcddd..933804b171a 100644 --- a/Compiler/preproc/preprocessor.cpp +++ b/Compiler/preproc/preprocessor.cpp @@ -245,7 +245,9 @@ namespace Preprocessor { size_t endOfString = FindIndexOfMatchingCharacter(text, i, text[i]); if (endOfString == NOT_FOUND) //size_t is unsigned but it's alright { - LogError(ErrorCode::UnterminatedString, "Unterminated string"); + std::string msg = "Unterminated string: is missing"; + msg = msg.replace(msg.find(""), 6, std::string(1, text[i])); + LogError(ErrorCode::UnterminatedString, msg.c_str()); break; } endOfString++; @@ -405,12 +407,24 @@ namespace Preprocessor { { if ((line[i] == '"') || (line[i] == '\'')) { - i = FindIndexOfMatchingCharacter(line, i, line[i]); - if (i == NOT_FOUND) + int end_of_literal = FindIndexOfMatchingCharacter(line, i, line[i]); + if (end_of_literal == NOT_FOUND) { i = line.GetLength(); break; } + if (i == 0u && line[0u] == '"') + { + // '[end_of_literal]' contains the '"', we need the part before that + String literal = line.Mid(0u, end_of_literal); + if (literal.StartsWith(NEW_SCRIPT_TOKEN_PREFIX)) + { + // Start the new script + _scriptName = literal.Mid(strlen(NEW_SCRIPT_TOKEN_PREFIX)); + _lineNumber = 0; + } + } + i = end_of_literal; } i++; } @@ -510,4 +524,4 @@ namespace Preprocessor { } } // Preprocessor -} // AGS \ No newline at end of file +} // AGS From 0ac86805a3c43a577b74c3ff483598c61429b3cc Mon Sep 17 00:00:00 2001 From: "Peter G Bouillon [fernewelten]" Date: Thu, 12 Dec 2024 03:24:33 +0100 Subject: [PATCH 3/5] add #include --- Compiler/preproc/preprocessor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Compiler/preproc/preprocessor.cpp b/Compiler/preproc/preprocessor.cpp index 933804b171a..8db8cd26fa7 100644 --- a/Compiler/preproc/preprocessor.cpp +++ b/Compiler/preproc/preprocessor.cpp @@ -13,6 +13,7 @@ //============================================================================= #include #include +#include #include "script/cs_parser_common.h" #include "preproc/preprocessor.h" #include "script/cc_common.h" From 93b43c6279d57957237a4abeda1f169b3a7e049d Mon Sep 17 00:00:00 2001 From: "Peter G Bouillon [fernewelten]" Date: Tue, 17 Dec 2024 16:53:32 +0100 Subject: [PATCH 4/5] Googletests and fixes for 'preprocessor.cpp' - Include 'string.h' instead of 'string' - Delete external 'currentline' declaration, is already in 'cc_common'' - Introduce static buffer for script name for use in 'cc_error()' - Set 'currentline' and 'ccCurScriptName' before 'cc_error()' is called, and _only_ there - Use 'String::FromFormat()' for formatting the error messages Googletests: - Add external reference to 'ccCurScriptName' - Add googletests for multiple scripts in one file; unterminated strings - Add conversion to shut up 'signed/unsigned' warning --- Compiler/preproc/preprocessor.cpp | 18 ++++--- Compiler/preproc/preprocessor.h | 11 ++++- Compiler/test/cc_test_helper.cpp | 2 - Compiler/test/cc_test_helper.h | 5 +- Compiler/test/preprocessor_test.cpp | 74 ++++++++++++++++++++++++++++- 5 files changed, 96 insertions(+), 14 deletions(-) diff --git a/Compiler/preproc/preprocessor.cpp b/Compiler/preproc/preprocessor.cpp index 8db8cd26fa7..bf2a22a027a 100644 --- a/Compiler/preproc/preprocessor.cpp +++ b/Compiler/preproc/preprocessor.cpp @@ -13,7 +13,7 @@ //============================================================================= #include #include -#include +#include #include "script/cs_parser_common.h" #include "preproc/preprocessor.h" #include "script/cc_common.h" @@ -25,7 +25,6 @@ using namespace AGS::Common; -extern int currentline; // in script/script_common namespace AGS { namespace Preprocessor { @@ -39,6 +38,7 @@ namespace Preprocessor { #endif const Error Preprocessor::NoError = Error(); + String Preprocessor::_scriptOfError = {}; size_t FindIndexOfMatchingCharacter(String text, size_t indexOfFirstSpeechMark, int charToMatch) { @@ -149,6 +149,11 @@ namespace Preprocessor { err.Message = msg; _errors.push_back(err); + // 'cc_error()' will only work properly when the global variables + // 'currentline' and 'ccCurScriptName' are current + currentline = _lineNumber; + _scriptOfError = _scriptName; + ccCurScriptName = _scriptOfError.GetCStr(); cc_error(msg.GetCStr()); } @@ -246,9 +251,8 @@ namespace Preprocessor { size_t endOfString = FindIndexOfMatchingCharacter(text, i, text[i]); if (endOfString == NOT_FOUND) //size_t is unsigned but it's alright { - std::string msg = "Unterminated string: is missing"; - msg = msg.replace(msg.find(""), 6, std::string(1, text[i])); - LogError(ErrorCode::UnterminatedString, msg.c_str()); + String msg = String::FromFormat("Unterminated string: '%c' is missing", text[i]); + LogError(ErrorCode::UnterminatedString, msg); break; } endOfString++; @@ -478,7 +482,7 @@ namespace Preprocessor { { _errors.clear(); StringBuilder output = StringBuilder(script.GetLength()); - currentline = _lineNumber = 0; + _lineNumber = 0; String escapedScriptName = scriptName; escapedScriptName.Replace("\\", "\\\\"); output.WriteLine(String::FromFormat("%s%s\"", NEW_SCRIPT_TOKEN_PREFIX, escapedScriptName.GetCStr())); @@ -486,7 +490,7 @@ namespace Preprocessor { _scriptName = scriptName; while (reader.IsValid()) { - currentline = ++_lineNumber; + ++_lineNumber; String thisLine = reader.ReadLine(); thisLine = RemoveComments(thisLine); if (thisLine.GetLength() > 0) diff --git a/Compiler/preproc/preprocessor.h b/Compiler/preproc/preprocessor.h index 957ad082fbd..341c32d7aff 100644 --- a/Compiler/preproc/preprocessor.h +++ b/Compiler/preproc/preprocessor.h @@ -49,6 +49,15 @@ namespace Preprocessor { int _lineNumber; String _scriptName; Version _applicationVersion; + // A 'static' buffer for the script name when an error is + // reported with 'cc_error()'. We can't use the non-'static' + // field '_scriptName' for this purpose because the + // 'cc_error()' mechanism will need the buffer after this + // 'Preprocessor' object has been destroyed. + // TODO: Do away with '_scriptOfError' when 'Preprocessor' + // is refactored so that it doesn't use 'cc_error()' for + // error reporting + static String _scriptOfError; // Conditional statement stack remembers the results of all the nested conditions // that we have entered. std::stack _conditionalStatements; @@ -82,4 +91,4 @@ namespace Preprocessor { }; } // Preprocessor -} // AGS \ No newline at end of file +} // AGS diff --git a/Compiler/test/cc_test_helper.cpp b/Compiler/test/cc_test_helper.cpp index 836bb69b7fd..55e2d6c41bd 100644 --- a/Compiler/test/cc_test_helper.cpp +++ b/Compiler/test/cc_test_helper.cpp @@ -15,8 +15,6 @@ #include "util/string_compat.h" #include "util/string.h" -extern int currentline; // in script/script_common - typedef AGS::Common::String AGSString; std::string last_cc_error_buf; diff --git a/Compiler/test/cc_test_helper.h b/Compiler/test/cc_test_helper.h index 930014ca49a..0645cd58f5b 100644 --- a/Compiler/test/cc_test_helper.h +++ b/Compiler/test/cc_test_helper.h @@ -25,5 +25,6 @@ extern void clear_error(void); extern const char *last_seen_cc_error(void); extern std::pair cc_error_at_line(const char* error_msg); extern AGS::Common::String cc_error_without_line(const char* error_msg); - -#endif // __CC_TEST_HELPER_H \ No newline at end of file +extern int currentline; +extern const char *ccCurScriptName; +#endif // __CC_TEST_HELPER_H diff --git a/Compiler/test/preprocessor_test.cpp b/Compiler/test/preprocessor_test.cpp index d26c238a635..c981762f57c 100644 --- a/Compiler/test/preprocessor_test.cpp +++ b/Compiler/test/preprocessor_test.cpp @@ -25,7 +25,7 @@ namespace Preprocessor { std::vector SplitLines(const AGSString& str) { std::vector str_lines = str.Split('\n'); - for (int i = 0; i < str_lines.size(); i++) { + for (int i = 0; i < (int) str_lines.size(); i++) { if (str_lines[i].CompareRight("\r", 1) == 0) { str_lines[i].ClipRight(1); } @@ -812,5 +812,75 @@ void FuncИह€한𐍈() { ASSERT_EQ(pp.GetLastError().Type, ErrorCode::InvalidCharacter); } +TEST(Preprocess, UnterminatedStringSingleQuote) { + Preprocessor pp = Preprocessor(); + const char *inpl = R"EOS( +int Func1() +{ + 'unterminated string + return 0; +} +)EOS"; + + clear_error(); + String res = pp.Preprocess(inpl, "UnterminatedStringSingleQuote"); + + // According to the googletest docs, + // the expected value should come first, the tested expression second + // Then in the case of a failed test, the reported message comes out correctly. + EXPECT_STREQ("Unterminated string: ''' is missing", last_seen_cc_error()); + auto err = pp.GetLastError(); + // script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc + EXPECT_EQ(4, currentline); +} + +TEST(Preprocess, UnterminatedStringDoubleQuote) { + Preprocessor pp = Preprocessor(); + const char *inpl = R"EOS( +int Func1() +{ + "unterminated string + return 0; +} +)EOS"; + + clear_error(); + String res = pp.Preprocess(inpl, "UnterminatedStringDoubleQuote"); + + EXPECT_STREQ(last_seen_cc_error(), "Unterminated string: '\"' is missing"); + // script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc + EXPECT_EQ(currentline, 4); +} + +TEST(Preprocess, MultipleNewScriptMarkers) { + Preprocessor pp = Preprocessor(); + const char *inpl = R"EOS( +"__NEWSCRIPTSTART_Dialog1" +int Func1() +{ + return 0; +} +"__NEWSCRIPTSTART_Dialog2" +int Func2() +{ + "unterminated string + return 0; +} +"__NEWSCRIPTSTART_Dialog3" +int Func3() +{ + return 0; +} +)EOS"; + + clear_error(); + String res = pp.Preprocess(inpl, "MultipleNewScriptMarkers"); + + EXPECT_STREQ("Unterminated string: '\"' is missing", last_seen_cc_error()); + EXPECT_STREQ("Dialog2", ccCurScriptName); + // Line count starts from the nearest NEW SCRIPT MARKER + EXPECT_EQ(3, currentline); +} + } // Preprocessor -} // AGS \ No newline at end of file +} // AGS From 3f82698a74e89594d0b0916daef9cc3283383501 Mon Sep 17 00:00:00 2001 From: "Peter G Bouillon [fernewelten]" Date: Tue, 17 Dec 2024 17:36:52 +0100 Subject: [PATCH 5/5] Do away with '_scriptOfError' Also, remove '(int)' cast in favour of 'size_t' loop var --- Compiler/preproc/preprocessor.cpp | 4 +--- Compiler/preproc/preprocessor.h | 9 --------- Compiler/test/cc_test_helper.h | 2 +- Compiler/test/preprocessor_test.cpp | 4 ++-- 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/Compiler/preproc/preprocessor.cpp b/Compiler/preproc/preprocessor.cpp index bf2a22a027a..65cba280114 100644 --- a/Compiler/preproc/preprocessor.cpp +++ b/Compiler/preproc/preprocessor.cpp @@ -38,7 +38,6 @@ namespace Preprocessor { #endif const Error Preprocessor::NoError = Error(); - String Preprocessor::_scriptOfError = {}; size_t FindIndexOfMatchingCharacter(String text, size_t indexOfFirstSpeechMark, int charToMatch) { @@ -152,8 +151,7 @@ namespace Preprocessor { // 'cc_error()' will only work properly when the global variables // 'currentline' and 'ccCurScriptName' are current currentline = _lineNumber; - _scriptOfError = _scriptName; - ccCurScriptName = _scriptOfError.GetCStr(); + ccCurScriptName = _scriptName.GetCStr(); cc_error(msg.GetCStr()); } diff --git a/Compiler/preproc/preprocessor.h b/Compiler/preproc/preprocessor.h index 341c32d7aff..5e94437bbb7 100644 --- a/Compiler/preproc/preprocessor.h +++ b/Compiler/preproc/preprocessor.h @@ -49,15 +49,6 @@ namespace Preprocessor { int _lineNumber; String _scriptName; Version _applicationVersion; - // A 'static' buffer for the script name when an error is - // reported with 'cc_error()'. We can't use the non-'static' - // field '_scriptName' for this purpose because the - // 'cc_error()' mechanism will need the buffer after this - // 'Preprocessor' object has been destroyed. - // TODO: Do away with '_scriptOfError' when 'Preprocessor' - // is refactored so that it doesn't use 'cc_error()' for - // error reporting - static String _scriptOfError; // Conditional statement stack remembers the results of all the nested conditions // that we have entered. std::stack _conditionalStatements; diff --git a/Compiler/test/cc_test_helper.h b/Compiler/test/cc_test_helper.h index 0645cd58f5b..3c2edbfb145 100644 --- a/Compiler/test/cc_test_helper.h +++ b/Compiler/test/cc_test_helper.h @@ -26,5 +26,5 @@ extern const char *last_seen_cc_error(void); extern std::pair cc_error_at_line(const char* error_msg); extern AGS::Common::String cc_error_without_line(const char* error_msg); extern int currentline; -extern const char *ccCurScriptName; +extern std::string ccCurScriptName; #endif // __CC_TEST_HELPER_H diff --git a/Compiler/test/preprocessor_test.cpp b/Compiler/test/preprocessor_test.cpp index c981762f57c..729fb0d3713 100644 --- a/Compiler/test/preprocessor_test.cpp +++ b/Compiler/test/preprocessor_test.cpp @@ -25,7 +25,7 @@ namespace Preprocessor { std::vector SplitLines(const AGSString& str) { std::vector str_lines = str.Split('\n'); - for (int i = 0; i < (int) str_lines.size(); i++) { + for (size_t i = 0; i < str_lines.size(); i++) { if (str_lines[i].CompareRight("\r", 1) == 0) { str_lines[i].ClipRight(1); } @@ -877,7 +877,7 @@ int Func3() String res = pp.Preprocess(inpl, "MultipleNewScriptMarkers"); EXPECT_STREQ("Unterminated string: '\"' is missing", last_seen_cc_error()); - EXPECT_STREQ("Dialog2", ccCurScriptName); + EXPECT_STREQ("Dialog2", ccCurScriptName.c_str()); // Line count starts from the nearest NEW SCRIPT MARKER EXPECT_EQ(3, currentline); }