diff --git a/CMakeLists.txt b/CMakeLists.txt index dbfec7045..8df466cb5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1062,6 +1062,7 @@ src/script/C4AulParse.cpp src/script/C4AulParse.h src/script/C4AulScriptFunc.cpp src/script/C4AulScriptFunc.h +src/script/C4AulWarnings.h src/script/C4Effect.cpp src/script/C4Effect.h src/script/C4PropList.cpp diff --git a/docs/clonk.xsl b/docs/clonk.xsl index cb9a044a3..ce32a5363 100644 --- a/docs/clonk.xsl +++ b/docs/clonk.xsl @@ -516,7 +516,7 @@ - + diff --git a/docs/sdk/script/Diagnostics.xml b/docs/sdk/script/Diagnostics.xml new file mode 100644 index 000000000..246bd90da --- /dev/null +++ b/docs/sdk/script/Diagnostics.xml @@ -0,0 +1,251 @@ + + + + + Diagnostic Messages + Diagnostic Messages + + + Certain constructs may be flagged by the engine as potentially + unintended or deprecated. In these cases, the engine will, by default, + emit a warning to the log file. + + + On occasion, these constructs are in fact intended by the script + author. In order to avoid unwanted warning messages hiding more important + messages, the engine supports selectively suppressing a warning category + for parts of a script. + + + + + Suppression and re-enablement is handled by the #warning + directive. The directive must be placed on a separate line. + + Warnings can be controlled using this syntax: + #warning {enable|disable} [warning_category [warning_category...]] + + If no category is given, the engine will suppress or enable all messages, + including those that are not enabled by default. A category remains + disabled or enabled until the next directive that affects it, or until + the end of the script. A script linked to via the #include + or #appendto directives does not affect, and is itself not + affected by, the warning settings of the current script. + + + It is not an error to specify a category that does not exist; the + invalid category is simply ignored. No separate warning is emitted. + + + + Warning Categories + The following warning categories currently exist: + + + Category + Description + + + invalid_escape_sequence + + + The engine found an escape sequence inside a string that it + did not recognize. + + "\p" + + + + invalid_hex_escape + + + The engine found the start of a hexadecimal escape sequence + inside a string, but no hexadecimal digits followed it. + + "\xGN" + + + + type_name_used_as_par_name + + + A function parameter was declared without an explicit type + specification, but with a name that is the same as a built-in type. + + func f(array) + + This warning is not enabled by default. + ¹ + + + + + empty_parameter_in_call + + + In a function call, a parameter was left empty. The engine is + passing nil in its place. + + CreateObject(Clonk,, 30, 100); + + This warning is not enabled by default. + ¹ + + + + + empty_parameter_in_array + + + In an array literal, an entry was left empty. The engine is + using nil in its place. + + [1, 2,, 3, 4] + + This warning is not enabled by default. + ¹ + + + + + implicit_range_loop_var_decl + + + The loop variable of a for-in loop was not declared either in the + loop header itself nor in the containing function. This is only + accepted for backwards compatibility and may be removed in a + future release. Explicitly declare the variable by adding the + var keyword. + + + func f() { + for (i in [1, 2, 3]) { + } +} + + + + + non_global_var_is_never_const + + + A variable has been declared as const, but is not + global. At this time, non-global variables are always mutable. + + + const local a = {} + + + + + variable_shadows_variable + + + The declaration of a variable uses the same name as a variable + in a greater scope. Changes to the shadowing variable will not + affect the shadowed variable. + + + static foo; +func f() { + var foo = 3; +} + + + + + redeclaration + + + A variable has been redeclared in the same scope. Make sure + you do not accidentally overwrite values another part of the + code relies upon. + + + func f() { + var i; + var i; +} + + + + + undeclared_varargs + + + Use of Par inside a function + implicitly declares it as using a variable number of arguments. + This is not immediately obvious to callers of the function, and + should be explicitly declared in the function signature by + adding a final ... parameter. + + + func f(a) { + return Par(a); +} +// Better: +func g(a, ...) { + return Par(a); +} + + + + + arg_count_mismatch + + + A function call passes more parameters than the function will + accept. + + + GetDir(0) + + + + + arg_type_mismatch + + + The parameter given in a function call is of a different type + than the called function expects. The call will likely fail at + runtime. + + + Sin("huh?") + + + + + empty_if + + + An if conditional is controlling an empty statement. + Use the empty block {} if this is intentional, or + remove the conditional entirely. + + + if (true); + + + +
+ + + + Examples + + + func f(string s) { + Sin(s); // WARNING: parameter 0 of call to 'Sin' passes string (int expected) +#warning disable arg_type_mismatch + Sin(s); +#warning enable arg_type_mismatch + Sin(s); // WARNING: parameter 0 of call to 'Sin' passes string (int expected) +} + + + + + ¹ The warning may be enabled by default in a future version. + diff --git a/docs/sdk/script/index.xml b/docs/sdk/script/index.xml index f9dd83669..c8415f396 100644 --- a/docs/sdk/script/index.xml +++ b/docs/sdk/script/index.xml @@ -47,6 +47,7 @@ Querying Game Data Script Player (i.e. AI player) Sound modifiers + Diagnostic messages Libraries Shape diff --git a/src/config/C4Config.cpp b/src/config/C4Config.cpp index 4b058a8cb..33853d558 100644 --- a/src/config/C4Config.cpp +++ b/src/config/C4Config.cpp @@ -88,7 +88,6 @@ void C4ConfigGeneral::CompileFunc(StdCompiler *pComp) void C4ConfigDeveloper::CompileFunc(StdCompiler *pComp) { pComp->Value(mkNamingAdapt(AutoFileReload, "AutoFileReload", 1 , false, true)); - pComp->Value(mkNamingAdapt(ExtraWarnings, "ExtraWarnings", 0 , false, true)); pComp->Value(mkNamingAdapt(s(TodoFilename), "TodoFilename", "{SCENARIO}/TODO.txt", false, true)); pComp->Value(mkNamingAdapt(s(AltTodoFilename), "AltTodoFilename2", "{USERPATH}/TODO.txt", false, true)); pComp->Value(mkNamingAdapt(MaxScriptMRU, "MaxScriptMRU", 30 , false, false)); diff --git a/src/config/C4Config.h b/src/config/C4Config.h index 98b830d0e..fb4ba7f7d 100644 --- a/src/config/C4Config.h +++ b/src/config/C4Config.h @@ -82,7 +82,6 @@ class C4ConfigDeveloper { public: int32_t AutoFileReload; - int32_t ExtraWarnings; char TodoFilename[CFG_MaxString + 1]; char AltTodoFilename[CFG_MaxString + 1]; int32_t MaxScriptMRU; // maximum number of remembered elements in recently used scripts diff --git a/src/lib/StdBuf.cpp b/src/lib/StdBuf.cpp index 44369245b..6e89e8ae1 100644 --- a/src/lib/StdBuf.cpp +++ b/src/lib/StdBuf.cpp @@ -24,7 +24,7 @@ #ifdef _WIN32 #include #include "platform/C4windowswrapper.h" -#define vsnprintf _vsnprintf +#define vsnprintf _vsprintf_p #else #define O_BINARY 0 #define O_SEQUENTIAL 0 diff --git a/src/script/C4Aul.cpp b/src/script/C4Aul.cpp index acb722265..e7c4f718c 100644 --- a/src/script/C4Aul.cpp +++ b/src/script/C4Aul.cpp @@ -17,6 +17,7 @@ #include "C4Include.h" #include "script/C4Aul.h" + #include "script/C4AulExec.h" #include "script/C4AulDebug.h" #include "config/C4Config.h" @@ -26,6 +27,22 @@ #include "c4group/C4Components.h" #include "c4group/C4LangStringTable.h" +const char *C4AulWarningMessages[] = { +#define DIAG(id, text, enabled) text, +#include "C4AulWarnings.h" +#undef DIAG + nullptr +}; +const char *C4AulWarningIDs[] = { +#define DIAG(id, text, enabled) #id, +#include "C4AulWarnings.h" +#undef DIAG + nullptr +}; + +static_assert(std::extent::value - 1 == static_cast(C4AulWarningId::WarningCount), "Warning message count doesn't match warning count"); +static_assert(std::extent::value - 1 == static_cast(C4AulWarningId::WarningCount), "Warning ID count doesn't match warning count"); + static class DefaultErrorHandler : public C4AulErrorHandler { public: diff --git a/src/script/C4Aul.h b/src/script/C4Aul.h index bca6d1cd3..4bded1fb4 100644 --- a/src/script/C4Aul.h +++ b/src/script/C4Aul.h @@ -27,6 +27,18 @@ // consts #define C4AUL_MAX_Identifier 100 // max length of function identifiers +// warning flags +enum class C4AulWarningId +{ +#define DIAG(id, text, enabled) id, +#include "C4AulWarnings.h" +#undef DIAG + WarningCount +}; + +extern const char *C4AulWarningIDs[]; +extern const char *C4AulWarningMessages[]; + // generic C4Aul error class class C4AulError : public std::exception { diff --git a/src/script/C4AulCompiler.cpp b/src/script/C4AulCompiler.cpp index 10067e293..6840e359c 100644 --- a/src/script/C4AulCompiler.cpp +++ b/src/script/C4AulCompiler.cpp @@ -29,6 +29,10 @@ #define C4AUL_SafeInherited "_inherited" #define C4AUL_DebugBreak "__debugbreak" +#ifdef _MSC_VER +#define vsnprintf _vsprintf_p +#endif + static std::string vstrprintf(const char *format, va_list args) { va_list argcopy; @@ -86,23 +90,30 @@ static std::string FormatCodePosition(const C4ScriptHost *source_host, const cha } template -static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const char *SPos, const C4AulScriptFunc *func, const char *msg, T &&...args) +static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const char *SPos, const C4AulScriptFunc *func, C4AulWarningId warning, T &&...args) { + if (!host->IsWarningEnabled(SPos, warning)) + return; + const char *msg = C4AulWarningMessages[static_cast(warning)]; std::string message = sizeof...(T) > 0 ? strprintf(msg, std::forward(args)...) : msg; message += FormatCodePosition(host, SPos, target_host, func); + message += " ["; + message += C4AulWarningIDs[static_cast(warning)]; + message += ']'; + ::ScriptEngine.GetErrorHandler()->OnWarning(message.c_str()); } template -static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const ::aul::ast::Node *n, const C4AulScriptFunc *func, const char *msg, T &&...args) +static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const ::aul::ast::Node *n, const C4AulScriptFunc *func, C4AulWarningId warning, T &&...args) { - return Warn(target_host, host, n->loc, func, msg, std::forward(args)...); + return Warn(target_host, host, n->loc, func, warning, std::forward(args)...); } template -static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const std::nullptr_t &, const C4AulScriptFunc *func, const char *msg, T &&...args) +static void Warn(const C4ScriptHost *target_host, const C4ScriptHost *host, const std::nullptr_t &, const C4AulScriptFunc *func, C4AulWarningId warning, T &&...args) { - return Warn(target_host, host, static_cast(nullptr), func, msg, std::forward(args)...); + return Warn(target_host, host, static_cast(nullptr), func, warning, std::forward(args)...); } template @@ -463,7 +474,7 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::RangeLoop *n) // the function and warn if it hasn't been declared at all. if (Fn->VarNamed.GetItemNr(cname) == -1) { - Warn(target_host, host, n, Fn, "Implicit declaration of the loop variable in a for-in loop is deprecated: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::implicit_range_loop_var_decl, cname); Fn->VarNamed.AddName(cname); } } @@ -474,7 +485,7 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::VarDecl *n) { if (n->constant && n->scope != ::aul::ast::VarDecl::Scope::Global) { - Warn(target_host, host, n, Fn, "Non-global variables cannot be constant"); + Warn(target_host, host, n, Fn, C4AulWarningId::non_global_var_is_never_const); } for (const auto &var : n->decls) { @@ -492,10 +503,10 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::VarDecl *n) // if target_host is unset, we're parsing this func for direct execution, // in which case we don't want to warn about variable hiding. if (target_host->Engine->GlobalNamedNames.GetItemNr(cname) >= 0 || target_host->Engine->GlobalConstNames.GetItemNr(cname) >= 0) - Warn(target_host, host, n, Fn, "function-local variable hides a global variable: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::variable_shadows_variable, cname, "local variable", "global variable"); C4String *s = ::Strings.FindString(cname); if (s && target_host->GetPropList()->HasProperty(s)) - Warn(target_host, host, n, Fn, "function-local variable hides an object-local variable: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::variable_shadows_variable, cname, "local variable", "object-local variable"); } Fn->VarNamed.AddName(cname); break; @@ -503,10 +514,10 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::VarDecl *n) case ::aul::ast::VarDecl::Scope::Object: { if (host->Engine->GlobalNamedNames.GetItemNr(cname) >= 0 || host->Engine->GlobalConstNames.GetItemNr(cname) >= 0) - Warn(target_host, host, n, Fn, "object-local variable hides a global variable: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::variable_shadows_variable, cname, "object-local variable", "global variable"); C4String *s = ::Strings.RegString(cname); if (target_host->GetPropList()->HasProperty(s)) - Warn(target_host, host, n, Fn, "object-local variable declared multiple times: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::redeclaration, cname, "object-local variable"); else target_host->GetPropList()->SetPropertyByS(s, C4VNull); break; @@ -517,7 +528,7 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::VarDecl *n) throw Error(target_host, host, n, Fn, "internal error: global var declaration inside function"); if (host->Engine->GlobalNamedNames.GetItemNr(cname) >= 0 || host->Engine->GlobalConstNames.GetItemNr(cname) >= 0) - Warn(target_host, host, n, Fn, "global variable declared multiple times: %s", cname); + Warn(target_host, host, n, Fn, C4AulWarningId::redeclaration, cname, "global variable"); if (n->constant) host->Engine->GlobalConstNames.AddName(cname); else @@ -584,7 +595,7 @@ void C4AulCompiler::PreparseAstVisitor::visit(const ::aul::ast::ParExpr *n) { if (Fn->ParCount != C4AUL_MAX_Par) { - Warn(target_host, host, n, Fn, "using Par() inside a function forces it to take variable arguments"); + Warn(target_host, host, n, Fn, C4AulWarningId::undeclared_varargs, "Par()"); Fn->ParCount = C4AUL_MAX_Par; } DefaultRecursiveVisitor::visit(n); @@ -1288,8 +1299,8 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::CallExpr *n) if (n->args.size() > fn_argc) { // Pop off any args that are over the limit - Warn(target_host, host, n->args[fn_argc].get(), Fn, - "call to %s passes %zu parameters, of which only %zu are used", cname, n->args.size(), fn_argc); + Warn(target_host, host, n->args[fn_argc].get(), Fn, C4AulWarningId::arg_count_mismatch, + cname, n->args.size(), fn_argc); AddBCC(n->loc, AB_STACK, fn_argc - n->args.size()); } else if (n->args.size() < fn_argc) @@ -1369,7 +1380,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::CallExpr *n) C4V_Type to = expected_par_types[i]; if (C4Value::WarnAboutConversion(from, to)) { - Warn(target_host, host, n->args[i].get(), Fn, "parameter %zu of %s is %s (%s expected)", i, cname, GetC4VName(from), GetC4VName(to)); + Warn(target_host, host, n->args[i].get(), Fn, C4AulWarningId::arg_type_mismatch, cname, i, GetC4VName(from), GetC4VName(to)); } } @@ -1576,14 +1587,14 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::If *n) // Warn if we're controlling a no-op ("if (...);") if (dynamic_cast<::aul::ast::Noop*>(n->iftrue.get())) { - Warn(target_host, host, n->iftrue->loc, Fn, "empty controlled statement found (use '{}' if this is intentional)"); + Warn(target_host, host, n->iftrue->loc, Fn, C4AulWarningId::empty_if); } if (SafeVisit(n->iftrue)) MaybePopValueOf(n->iftrue); if (dynamic_cast<::aul::ast::Noop*>(n->iffalse.get())) { - Warn(target_host, host, n->iffalse->loc, Fn, "empty controlled statement found (use '{}' if this is intentional)"); + Warn(target_host, host, n->iffalse->loc, Fn, C4AulWarningId::empty_if); } if (n->iffalse) @@ -1642,7 +1653,7 @@ void C4AulCompiler::CodegenAstVisitor::visit(const ::aul::ast::FunctionDecl *n) if (f->SFunc() && f->SFunc()->pOrgScript == host && f->Parent == Parent) { if (Fn) - Warn(target_host, host, n, Fn, "function declared multiple times"); + Warn(target_host, host, n, Fn, C4AulWarningId::redeclaration, f->GetName(), "function"); Fn = f->SFunc(); } f = f->SFunc() ? f->SFunc()->OwnerOverloaded : 0; diff --git a/src/script/C4AulParse.cpp b/src/script/C4AulParse.cpp index 6831973f6..3e7f593a6 100644 --- a/src/script/C4AulParse.cpp +++ b/src/script/C4AulParse.cpp @@ -38,6 +38,10 @@ #define C4AUL_Include "#include" #define C4AUL_Append "#appendto" +#define C4AUL_Warning "#warning" + +#define C4Aul_Warning_enable "enable" +#define C4Aul_Warning_disable "disable" #define C4AUL_Func "func" @@ -128,19 +132,20 @@ C4AulParse::~C4AulParse() void C4ScriptHost::Warn(const char *pMsg, ...) { va_list args; va_start(args, pMsg); - StdStrBuf Buf; - Buf.AppendFormatV(pMsg, args); + StdStrBuf Buf = FormatStringV(pMsg, args); Buf.AppendFormat(" (%s)", ScriptName.getData()); Engine->GetErrorHandler()->OnWarning(Buf.getData()); va_end(args); } -void C4AulParse::Warn(const char *pMsg, ...) +void C4AulParse::Warn(C4AulWarningId warning, ...) { - va_list args; va_start(args, pMsg); - StdStrBuf Buf; - Buf.AppendFormatV(pMsg, args); + if (!pOrgScript->IsWarningEnabled(TokenSPos, warning)) + return; + va_list args; va_start(args, warning); + StdStrBuf Buf = FormatStringV(C4AulWarningMessages[static_cast(warning)], args); AppendPosition(Buf); + Buf.AppendFormat(" [%s]", C4AulWarningIDs[static_cast(warning)]); Engine->GetErrorHandler()->OnWarning(Buf.getData()); va_end(args); } @@ -383,6 +388,16 @@ C4AulTokenType C4AulParse::GetNextToken() C = *(++SPos); } + // Special case for #warning because we don't want to give it to the parser + if (dir && SEqual2(TokenSPos, C4AUL_Warning)) + { + // Look for end of line or end of file + while (*SPos != '\n' && *SPos != '\0') ++SPos; + Parse_WarningPragma(); + // And actually return the next token. + return GetNextToken(); + } + Len = std::min(Len, C4AUL_MAX_Identifier); SCopy(TokenSPos, Idtf, Len); return dir ? ATT_DIR : ATT_IDTF; @@ -443,7 +458,7 @@ C4AulTokenType C4AulParse::GetNextToken() // First char must be a hexdigit if (!std::isxdigit(*SPos)) { - Warn("\\x used with no following hex digits"); + Warn(C4AulWarningId::invalid_hex_escape); strbuf.push_back('\\'); strbuf.push_back('x'); } else @@ -481,7 +496,7 @@ C4AulTokenType C4AulParse::GetNextToken() // just insert "\" strbuf.push_back('\\'); // show warning - Warn("unknown escape \"%c\"", *(SPos + 1)); + Warn(C4AulWarningId::invalid_escape_sequence, *(SPos + 1)); } } else if (C == 0 || C == 10 || C == 13) // line break / feed @@ -685,6 +700,13 @@ bool C4ScriptHost::Preparse() // Add any engine functions specific to this script AddEngineFunctions(); + // Insert default warnings + assert(enabledWarnings.empty()); + auto &warnings = enabledWarnings[Script.getData()]; +#define DIAG(id, text, enabled) warnings.set(static_cast(C4AulWarningId::id), enabled); +#include "C4AulWarnings.h" +#undef DIAG + C4AulParse parser(this); ast = parser.Parse_Script(this); @@ -749,6 +771,66 @@ void C4AulParse::UnexpectedToken(const char * Expected) throw C4AulParseError(this, FormatString("%s expected, but found %s", Expected, GetTokenName(TokenType)).getData()); } +void C4AulParse::Parse_WarningPragma() +{ + assert(SEqual2(TokenSPos, C4AUL_Warning)); + assert(std::isspace(TokenSPos[sizeof(C4AUL_Warning) - 1])); + + + // Read parameters in to string buffer. The sizeof() includes the terminating \0, but + // that's okay because we need to skip (at least) one whitespace character anyway. + std::string line(TokenSPos + sizeof(C4AUL_Warning), SPos); + auto end = line.end(); + auto cursor = std::find_if_not(begin(line), end, IsWhiteSpace); + + if (cursor == end) + throw C4AulParseError(this, "'" C4Aul_Warning_enable "' or '" C4Aul_Warning_disable "' expected, but found end of line"); + + // Split directive on whitespace + auto start = cursor; + cursor = std::find_if(start, end, IsWhiteSpace); + bool enable_warning = false; + if (std::equal(start, cursor, C4Aul_Warning_enable)) + { + enable_warning = true; + } + else if (std::equal(start, cursor, C4Aul_Warning_disable)) + { + enable_warning = false; + } + else + { + throw C4AulParseError(this, FormatString("'" C4Aul_Warning_enable "' or '" C4Aul_Warning_disable "' expected, but found '%s'", std::string(start, cursor).c_str()).getData()); + } + + cursor = std::find_if_not(cursor, end, IsWhiteSpace); + if (cursor == end) + { + // enable or disable all warnings +#define DIAG(id, text, enabled) pOrgScript->EnableWarning(TokenSPos, C4AulWarningId::id, enable_warning); +#include "C4AulWarnings.h" +#undef DIAG + return; + } + + // enable or disable specific warnings + static const std::map warnings{ +#define DIAG(id, text, enabled) std::make_pair(#id, C4AulWarningId::id), +#include "C4AulWarnings.h" +#undef DIAG + }; + while (cursor != end) + { + start = std::find_if_not(cursor, end, IsWhiteSpace); + cursor = std::find_if(start, end, IsWhiteSpace); + auto entry = warnings.find(std::string(start, cursor)); + if (entry != warnings.end()) + { + pOrgScript->EnableWarning(TokenSPos, entry->second, enable_warning); + } + } +} + void C4AulScriptFunc::ParseDirectExecFunc(C4AulScriptEngine *Engine, C4AulScriptContext* context) { ClearCode(); @@ -936,8 +1018,7 @@ void C4AulParse::Parse_Function(::aul::ast::Function *func) if (TokenType == ATT_BCLOSE || TokenType == ATT_COMMA) { par_name = Idtf; - if (Config.Developer.ExtraWarnings) - Warn("'%s' used as parameter name", Idtf); + Warn(C4AulWarningId::type_name_used_as_par_name, Idtf); } else { @@ -1097,8 +1178,7 @@ void C4AulParse::Parse_CallParams(::aul::ast::CallExpr *call) { case ATT_COMMA: // got no parameter before a "," - if (Config.Developer.ExtraWarnings) - Warn(FormatString("parameter %zu of call to %s is empty", call->args.size(), call->callee.c_str()).getData(), nullptr); + Warn(C4AulWarningId::empty_parameter_in_call, call->args.size(), call->callee.c_str()); call->args.push_back(::aul::ast::NilLit::New(TokenSPos)); Shift(); break; @@ -1131,8 +1211,7 @@ std::unique_ptr<::aul::ast::ArrayLit> C4AulParse::Parse_Array() // got no parameter before a ","? then push nil if (TokenType == ATT_COMMA) { - if (Config.Developer.ExtraWarnings) - Warn(FormatString("array entry %zu is empty", arr->values.size()).getData(), nullptr); + Warn(C4AulWarningId::empty_parameter_in_array, arr->values.size()); arr->values.emplace_back(::aul::ast::NilLit::New(TokenSPos)); } else @@ -1143,8 +1222,7 @@ std::unique_ptr<::aul::ast::ArrayLit> C4AulParse::Parse_Array() // [] -> size 0, [*,] -> size 2, [*,*,] -> size 3 if (TokenType == ATT_BCLOSE2) { - if (Config.Developer.ExtraWarnings) - Warn(FormatString("array entry %zu is empty", arr->values.size()).getData(), nullptr); + Warn(C4AulWarningId::empty_parameter_in_array, arr->values.size()); arr->values.emplace_back(::aul::ast::NilLit::New(TokenSPos)); } } @@ -1695,6 +1773,38 @@ bool C4ScriptHost::Parse() return true; } +void C4ScriptHost::EnableWarning(const char *pos, C4AulWarningId warning, bool enable) +{ + auto entry = enabledWarnings.emplace(pos, decltype(enabledWarnings)::mapped_type{}); + if (entry.second) + { + // If there was no earlier entry for this position, copy the previous + // warning state + assert(entry.first != enabledWarnings.begin()); + auto previous = entry.first; + --previous; + entry.first->second = previous->second; + } + entry.first->second.set(static_cast(warning), enable); +} + +bool C4ScriptHost::IsWarningEnabled(const char *pos, C4AulWarningId warning) const +{ + assert(!enabledWarnings.empty()); + if (enabledWarnings.empty()) + return false; + + // find nearest set of warnings at or before the current position + auto entry = enabledWarnings.upper_bound(pos); + assert(entry != enabledWarnings.begin()); + if (entry != enabledWarnings.begin()) + { + --entry; + } + + return entry->second.test(static_cast(warning)); +} + void C4AulParse::PushParsePos() { parse_pos_stack.push(TokenSPos); diff --git a/src/script/C4AulParse.h b/src/script/C4AulParse.h index 7e747cf92..4d2b3d6e5 100644 --- a/src/script/C4AulParse.h +++ b/src/script/C4AulParse.h @@ -62,6 +62,7 @@ private: C4String * cStr; // current string constant C4AulScriptContext* ContextToExecIn; void Parse_Function(bool parse_for_direct_exec); + void Parse_WarningPragma(); protected: // All of the Parse_* functions need to be protected (not private!) so @@ -93,7 +94,7 @@ private: void Check(C4AulTokenType TokenType, const char * Expected = nullptr); NORETURN void UnexpectedToken(const char * Expected); - void Warn(const char *pMsg, ...) GNUC_FORMAT_ATTRIBUTE_O; + void Warn(C4AulWarningId warning, ...); void Error(const char *pMsg, ...) GNUC_FORMAT_ATTRIBUTE_O; void AppendPosition(StdStrBuf & Buf); diff --git a/src/script/C4AulWarnings.h b/src/script/C4AulWarnings.h new file mode 100644 index 000000000..e6a888871 --- /dev/null +++ b/src/script/C4AulWarnings.h @@ -0,0 +1,42 @@ +/* +* OpenClonk, http://www.openclonk.org +* +* Copyright (c) 2017, The OpenClonk Team and contributors +* +* Distributed under the terms of the ISC license; see accompanying file +* "COPYING" for details. +* +* "Clonk" is a registered trademark of Matthes Bender, used with permission. +* See accompanying file "TRADEMARK" for details. +* +* To redistribute this file separately, substitute the full license texts +* for the above references. +*/ + +// C4Aul diagnostics definitions + +#pragma push_macro("DIAG") +#ifndef DIAG +#define DIAG(id, text, enabled_by_default) +#endif + +// Lexer diagnostics +DIAG(invalid_escape_sequence, "unknown escape sequence '\\%1$s'", true) +DIAG(invalid_hex_escape, "'\\x' used with no following hex digits", true) + +// Parser diagnostics +DIAG(type_name_used_as_par_name, "type '%1$s' used as parameter name", false) +DIAG(empty_parameter_in_call, "parameter %1$zu of call to '%2$s' is empty", false) +DIAG(empty_parameter_in_array, "array entry %1$zu is empty", false) + +// Compiler diagnostics +DIAG(implicit_range_loop_var_decl, "implicit declaration of the loop variable '%1$s' in a for-in loop is deprecated", true) +DIAG(non_global_var_is_never_const, "variable '%1$s' declared as const, but non-global variables are always mutable", true) +DIAG(variable_shadows_variable, "declaration of %2$s '%1$s' shadows %3$s", true) +DIAG(redeclaration, "redeclaration of %2$s '%1$s'", true) +DIAG(undeclared_varargs, "use of '%1$s' in a function forces it to take variable arguments", true) +DIAG(arg_count_mismatch, "call to '%1$s' passes %2$zu arguments, of which only %3$zu are used", true) +DIAG(arg_type_mismatch, "parameter %2$zu of call to '%1$s' passes %3$s (%4$s expected)", true) +DIAG(empty_if, "empty controlled statement (use '{}' if this is intentional)", true) + +#pragma pop_macro("DIAG") diff --git a/src/script/C4ScriptHost.cpp b/src/script/C4ScriptHost.cpp index 57ed3b943..a120dc623 100644 --- a/src/script/C4ScriptHost.cpp +++ b/src/script/C4ScriptHost.cpp @@ -71,6 +71,7 @@ void C4ScriptHost::Clear() Appends.clear(); // reset flags State = ASS_NONE; + enabledWarnings.clear(); } void C4ScriptHost::UnlinkOwnedFunctions() diff --git a/src/script/C4ScriptHost.h b/src/script/C4ScriptHost.h index e1bd25454..5be373b59 100644 --- a/src/script/C4ScriptHost.h +++ b/src/script/C4ScriptHost.h @@ -24,6 +24,7 @@ #include "script/C4Aul.h" #include "script/C4AulAST.h" +#include // aul script state enum C4AulScriptState @@ -55,6 +56,8 @@ public: std::list SourceScripts; StdCopyStrBuf ScriptName; // script name + bool IsWarningEnabled(const char *pos, C4AulWarningId warning) const; + protected: C4ScriptHost(); void Unreg(); // remove from list @@ -97,6 +100,8 @@ protected: // in Clear() even in case of cyclic references. std::vector ownedPropLists; + void EnableWarning(const char *pos, C4AulWarningId warning, bool enable = true); + friend class C4AulParse; friend class C4AulProfiler; friend class C4AulScriptEngine; @@ -105,6 +110,7 @@ protected: friend class C4AulScriptFunc; private: + std::map> enabledWarnings; std::unique_ptr<::aul::ast::Script> ast; }; diff --git a/tests/aul/AulTest.cpp b/tests/aul/AulTest.cpp index 4d9e6ffcc..14f51c858 100644 --- a/tests/aul/AulTest.cpp +++ b/tests/aul/AulTest.cpp @@ -313,3 +313,65 @@ TEST_F(AulTest, NoWarnings) EXPECT_CALL(errh, OnWarning(::testing::_)).Times(0); EXPECT_EQ(C4Value(), RunScript("func Main(string s, object o, array a) { var x; Sin(x); }")); } + +TEST_F(AulTest, DiagnosticsSelection) +{ + { + ErrorHandler errh; + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[arg_type_mismatch]"))).Times(2); + // Test disabling and re-enabling warnings + RunScript(R"( +func Main(string s) { + Sin(s); +#warning disable arg_type_mismatch + Sin(s); +#warning enable arg_type_mismatch + Sin(s); +} +)"); + } + { + ErrorHandler errh; + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[arg_type_mismatch]"))); + // Test that disabling a warning doesn't affect any other warnings + RunScript(R"( +func Main(string s) { +#warning disable redeclaration + Sin(s); +} +)"); + } + { + ErrorHandler errh; + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[arg_count_mismatch]"))).Times(0); + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[arg_type_mismatch]"))).Times(0); + // Test disabling multiple warnings at once + RunScript(R"( +func Main(string s) { +#warning disable arg_count_mismatch arg_type_mismatch + Sin(s, s); +} +)"); + } + { + ErrorHandler errh; + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[arg_type_mismatch]"))).Times(0); + // Test disabling all warnings at once + RunScript(R"( +func Main(string s) { +#warning disable + Sin(s); +} +)"); + } + { + ErrorHandler errh; + EXPECT_CALL(errh, OnWarning(::testing::EndsWith("[type_name_used_as_par_name]"))).Times(2); + // Test that disabled-by-default warnings have to be enabled explicitly + RunScript(R"( +func Main(array) {} +#warning enable type_name_used_as_par_name +func f(array, string) {} +)"); + } +} \ No newline at end of file