From 2f8f99bd94a30fb9ff80afbd3cd9823ed6da86ec Mon Sep 17 00:00:00 2001 From: Sylvie <35663410+Rangi42@users.noreply.github.com> Date: Mon, 5 Aug 2024 12:50:48 -0400 Subject: [PATCH] Implement `-Wpurge=` (#1443) --- contrib/bash_compl/_rgbasm.bash | 1 + contrib/zsh_compl/_rgbasm | 3 ++- include/asm/warning.hpp | 3 +++ man/rgbasm.1 | 12 ++++++++++++ man/rgbasm.5 | 17 ++++++++--------- src/asm/symbol.cpp | 4 ++++ src/asm/warning.cpp | 11 +++++++++++ test/asm/local-purge.err | 2 ++ test/asm/purge-multiple.err | 2 ++ test/asm/purge-ref.err | 4 ++++ test/asm/purge-refs.err | 2 ++ test/asm/purge.err | 4 ++++ test/asm/sym-collision.err | 2 ++ test/asm/trailing-commas.err | 2 ++ 14 files changed, 59 insertions(+), 10 deletions(-) create mode 100644 test/asm/purge-multiple.err diff --git a/contrib/bash_compl/_rgbasm.bash b/contrib/bash_compl/_rgbasm.bash index bf1f7a28..a2c5c1ab 100755 --- a/contrib/bash_compl/_rgbasm.bash +++ b/contrib/bash_compl/_rgbasm.bash @@ -187,6 +187,7 @@ _rgbasm_completions() { nested-comment numeric-string obsolete + purge shift shift-amount truncation diff --git a/contrib/zsh_compl/_rgbasm b/contrib/zsh_compl/_rgbasm index 36b1edac..fcb1bdbd 100644 --- a/contrib/zsh_compl/_rgbasm +++ b/contrib/zsh_compl/_rgbasm @@ -21,6 +21,7 @@ _rgbasm_warnings() { 'nested-comment:Warn on "/*" inside block comments' 'numeric-string:Warn when a multi-character string is treated as a number' 'obsolete:Warn when using deprecated features' + 'purge:Warn when purging exported symbols or labels' 'shift:Warn when shifting negative values' 'shift-amount:Warn when a shift'\''s operand it negative or \> 32' 'truncation:Warn when implicit truncation loses bits' @@ -28,7 +29,7 @@ _rgbasm_warnings() { 'user:Warn when executing the WARN built-in' ) # TODO: handle `no-` and `error=` somehow? - # TODO: handle `=0|1|2` levels for `numeric-string`, `truncation`, and `unmapped-char`? + # TODO: handle `=0|1|2` levels for `numeric-string`, `purge`, `truncation`, and `unmapped-char`? _describe warning warnings } diff --git a/include/asm/warning.hpp b/include/asm/warning.hpp index aef409d3..167de412 100644 --- a/include/asm/warning.hpp +++ b/include/asm/warning.hpp @@ -31,6 +31,9 @@ enum WarningID { // Treating string as number may lose some bits WARNING_NUMERIC_STRING_1 = PARAM_WARNINGS_START, WARNING_NUMERIC_STRING_2, + // Purging an exported symbol or label + WARNING_PURGE_1, + WARNING_PURGE_2, // Implicit truncation loses some bits WARNING_TRUNCATION_1, WARNING_TRUNCATION_2, diff --git a/man/rgbasm.1 b/man/rgbasm.1 index 6a6d7218..232a2793 100644 --- a/man/rgbasm.1 +++ b/man/rgbasm.1 @@ -309,6 +309,18 @@ or just warns about strings longer than four characters, since four or fewer characters fit within a 32-bit integer. .Fl Wnumeric-string=2 warns about any multi-character string. +.It Fl Wpurge= +Warn when purging symbols which are likely to have been necessary. +.Fl Wpurge=0 +or +.Fl Wno-purge +disables this warning. +.Fl Wpurge=1 +or just +.Fl Wpurge +warns when purging any exported symbol (regardless of type). +.Fl Wpurge=2 +also warns when purging any label (even if not exported). .It Fl Wshift Warn when shifting right a negative value. Use a division by 2**N instead. diff --git a/man/rgbasm.5 b/man/rgbasm.5 index d9afb6a9..754ffa8c 100644 --- a/man/rgbasm.5 +++ b/man/rgbasm.5 @@ -1383,16 +1383,15 @@ Note also that only exported symbols will appear in symbol and map files produce .Xr rgblink 1 . .Ss Purging symbols .Ic PURGE -allows you to completely remove a symbol from the symbol table as if it had never existed. -.Em USE WITH EXTREME CAUTION ! -I can't stress this enough, -.Sy you seriously need to know what you are doing . -DON'T purge a symbol that you use in expressions the linker needs to calculate. -When not sure, it's probably not safe to purge anything other than variables, numeric or string constants, or macros. +allows you to completely remove a symbol from the symbol table, as if it had never been defined. +Be +.Em very +careful when purging symbols, especially labels, because it could result in unpredictable errors if something depends on the missing symbol (for example, expressions the linker needs to calculate). .Bd -literal -offset indent -DEF Kamikaze EQUS "I don't want to live anymore" -DEF AOLer EQUS "Me too" - PURGE Kamikaze, AOLer +DEF Kamikaze EQUS "I don't want to live anymore" +AOLer: DB "Me too lol" +PURGE Kamikaze, AOLer +ASSERT !DEF(Kamikaze) && !DEF(AOLer) .Ed .Pp String constants are not expanded within the symbol names. diff --git a/src/asm/symbol.cpp b/src/asm/symbol.cpp index 46c12f01..ab60279b 100644 --- a/src/asm/symbol.cpp +++ b/src/asm/symbol.cpp @@ -169,6 +169,10 @@ void sym_Purge(std::string const &symName) { } else if (sym->ID != (uint32_t)-1) { error("Symbol \"%s\" is referenced and thus cannot be purged\n", symName.c_str()); } else { + if (sym->isExported) + warning(WARNING_PURGE_1, "Purging an exported symbol \"%s\"\n", symName.c_str()); + else if (sym->isLabel()) + warning(WARNING_PURGE_2, "Purging a label \"%s\"\n", symName.c_str()); // Do not keep a reference to the label's name after purging it if (sym->name == labelScope) labelScope = std::nullopt; diff --git a/src/asm/warning.cpp b/src/asm/warning.cpp index f4d6a01e..54b974e3 100644 --- a/src/asm/warning.cpp +++ b/src/asm/warning.cpp @@ -41,6 +41,8 @@ static WarningState const defaultWarnings[ARRAY_SIZE(warningStates)] = { WARNING_DISABLED, // WARNING_NUMERIC_STRING_2 WARNING_ENABLED, // WARNING_TRUNCATION_1 WARNING_DISABLED, // WARNING_TRUNCATION_2 + WARNING_ENABLED, // WARNING_PURGE_1 + WARNING_DISABLED, // WARNING_PURGE_2 WARNING_ENABLED, // WARNING_UNMAPPED_CHAR_1 WARNING_DISABLED, // WARNING_UNMAPPED_CHAR_2 }; @@ -87,6 +89,8 @@ static char const * const warningFlags[NB_WARNINGS] = { // Parametric warnings "numeric-string", "numeric-string", + "purge", + "purge", "truncation", "truncation", "unmapped-char", @@ -104,6 +108,7 @@ static const struct { uint8_t defaultLevel; } paramWarnings[] = { {"numeric-string", 2, 1}, + {"purge", 2, 1}, {"truncation", 2, 2}, {"unmapped-char", 2, 1}, }; @@ -161,6 +166,8 @@ static uint8_t const _wallCommands[] = { WARNING_LARGE_CONSTANT, WARNING_NESTED_COMMENT, WARNING_OBSOLETE, + WARNING_PURGE_1, + WARNING_PURGE_2, WARNING_UNMAPPED_CHAR_1, META_WARNING_DONE, }; @@ -171,6 +178,8 @@ static uint8_t const _wextraCommands[] = { WARNING_MACRO_SHIFT, WARNING_NESTED_COMMENT, WARNING_OBSOLETE, + WARNING_PURGE_1, + WARNING_PURGE_2, WARNING_TRUNCATION_1, WARNING_TRUNCATION_2, WARNING_UNMAPPED_CHAR_1, @@ -190,6 +199,8 @@ static uint8_t const _weverythingCommands[] = { WARNING_MACRO_SHIFT, WARNING_NESTED_COMMENT, WARNING_OBSOLETE, + WARNING_PURGE_1, + WARNING_PURGE_2, WARNING_SHIFT, WARNING_SHIFT_AMOUNT, WARNING_TRUNCATION_1, diff --git a/test/asm/local-purge.err b/test/asm/local-purge.err index 2eff165c..55064fd3 100644 --- a/test/asm/local-purge.err +++ b/test/asm/local-purge.err @@ -1,3 +1,5 @@ +warning: local-purge.asm(7): [-Wpurge] + Purging a label ".loc" error: local-purge.asm(8): Interpolated symbol ".loc" does not exist error: Assembly aborted (1 error)! diff --git a/test/asm/purge-multiple.err b/test/asm/purge-multiple.err new file mode 100644 index 00000000..2ddc1d10 --- /dev/null +++ b/test/asm/purge-multiple.err @@ -0,0 +1,2 @@ +warning: purge-multiple.asm(9): [-Wpurge] + Purging an exported symbol "u" diff --git a/test/asm/purge-ref.err b/test/asm/purge-ref.err index 6ef4bb99..b6a65c9c 100644 --- a/test/asm/purge-ref.err +++ b/test/asm/purge-ref.err @@ -1,7 +1,11 @@ error: purge-ref.asm(4): Symbol "ref" is referenced and thus cannot be purged +warning: purge-ref.asm(7): [-Wpurge] + Purging a label "OK" error: purge-ref.asm(11): Symbol "NotOK" is referenced and thus cannot be purged error: purge-ref.asm(15): Symbol "EvenLessOK" is referenced and thus cannot be purged +warning: purge-ref.asm(23): [-Wpurge] + Purging a label "Maybe" error: Assembly aborted (3 errors)! diff --git a/test/asm/purge-refs.err b/test/asm/purge-refs.err index d4761445..0ded8eb5 100644 --- a/test/asm/purge-refs.err +++ b/test/asm/purge-refs.err @@ -1,3 +1,5 @@ error: purge-refs.asm(6): Symbol "Floating" is referenced and thus cannot be purged +warning: purge-refs.asm(13): [-Wpurge] + Purging a label "Fixed" error: Assembly aborted (1 error)! diff --git a/test/asm/purge.err b/test/asm/purge.err index c6fb6599..8061c2d6 100644 --- a/test/asm/purge.err +++ b/test/asm/purge.err @@ -1,5 +1,9 @@ +warning: purge.asm(5): [-Wpurge] + Purging a label "Label" error: purge.asm(9): Symbol "Referenced" is referenced and thus cannot be purged +warning: purge.asm(12): [-Wpurge] + Purging an exported symbol "Exported" error: purge.asm(15): 'Undefined' not defined error: Assembly aborted (2 errors)! diff --git a/test/asm/sym-collision.err b/test/asm/sym-collision.err index 041c6e7d..7054acfd 100644 --- a/test/asm/sym-collision.err +++ b/test/asm/sym-collision.err @@ -1,3 +1,5 @@ +warning: sym-collision.asm(20): [-Wpurge] + Purging a label "dork" error: sym-collision.asm(25): Interpolated symbol "dork" does not exist error: Assembly aborted (1 error)! diff --git a/test/asm/trailing-commas.err b/test/asm/trailing-commas.err index 22f7d76d..57a10556 100644 --- a/test/asm/trailing-commas.err +++ b/test/asm/trailing-commas.err @@ -1,2 +1,4 @@ warning: trailing-commas.asm(7): [-Wempty-macro-arg] Empty macro argument +warning: trailing-commas.asm(22): [-Wpurge] + Purging a label "lobsterThermidor"