View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000584 | LDMud 3.3 | LPC Compiler/Preprocessor | public | 2008-12-03 02:24 | 2018-01-29 21:57 |
Reporter | _xtian_ | Assigned To | Gnomi | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | resolved | Resolution | fixed | ||
Platform | 64bit | OS | debian | OS Version | lenny |
Product Version | 3.3 | ||||
Fixed in Version | 3.3.718 | ||||
Summary | 0000584: variable definition in outer switch block | ||||
Description | It is possible to define a variable in the outer switch block (before the first case-statement). I find this uncommon an am not sure if this was intended. Also: This variable, if initialised before this first case-statement, will not be initialised correctly. Please see the following LPC code: | ||||
Steps To Reproduce | mixed test() { int i=1; switch(i) { mixed mx=2; // either: this definition shouldnt be allowed here case 1: // ... return mx; // or: this yield 0 } } | ||||
Tags | No tags attached. | ||||
Attached Files | bug584.diff (5,838 bytes)
Index: trunk.switch/test/t-language/inc =================================================================== --- trunk.switch/test/t-language/inc (Revision 0) +++ trunk.switch/test/t-language/inc (Revision 0) @@ -0,0 +1 @@ +link ../inc \ Kein Zeilenvorschub am Ende der Datei Eigenschaftsänderungen: trunk.switch/test/t-language/inc ___________________________________________________________________ Name: svn:special + * Index: trunk.switch/test/t-language/tf-switch1.c =================================================================== --- trunk.switch/test/t-language/tf-switch1.c (Revision 0) +++ trunk.switch/test/t-language/tf-switch1.c (Revision 0) @@ -0,0 +1,22 @@ +/* Switch test1 (Bug #584) + * + * The declaration of a variable within a switch block + * should not span over multiple case/default statements. + * (As it is in C++.) + */ + +int fun() +{ + switch(1) + { + case 0: + int i=1; + return i; + + case 1: + i=2; + return i; + } + + return -1; +} Index: trunk.switch/test/t-language/tf-switch2.c =================================================================== --- trunk.switch/test/t-language/tf-switch2.c (Revision 0) +++ trunk.switch/test/t-language/tf-switch2.c (Revision 0) @@ -0,0 +1,25 @@ +/* Switch test2 + * + * Duff's Device should not work. + * case/default should appear immediately within + * a switch block and not in an inner block. + * (As it is in Java.) + */ + +void fun() +{ + int count = 10; + int *src = allocate(10), *dest = allocate(10); + int n, pos; + + n = (count + 3) / 4; + + switch(count % 4) + { + case 0: do { dest[pos] = src[pos]; pos++; + case 3: dest[pos] = src[pos]; pos++; + case 2: dest[pos] = src[pos]; pos++; + case 1: dest[pos] = src[pos]; pos++; + } while(--n > 0); + } +} Index: trunk.switch/test/t-language/master.c =================================================================== --- trunk.switch/test/t-language/master.c (Revision 0) +++ trunk.switch/test/t-language/master.c (Revision 0) @@ -0,0 +1,66 @@ +/* LPC language tests. + * + * Tests, whether some files are compiled or not. + * Has a list tl-*.c of files that should load + * and a list tn-*.c of files that should not load. + * In the former run_test() is called and should + * return a non-zero value for success. + */ + +#include "/inc/base.inc" + +void run_test() +{ + int errors; + + msg("\nRunning test for t-language:\n" + "----------------------------\n"); + + foreach(string file: get_dir("/tl-*.c")) + { + string err; + int res; + + msg("Running Test %s...", file[0..<3]); + + if((err = catch(res = load_object(file[0..<3])->run_test();nolog))) + { + errors++; + msg(" FAILURE! (%s)\n", err[1..]); + } + else if(!res) + { + errors++; + msg(" FAILURE! (Wrong result)\n"); + } + else + { + msg(" Success.\n"); + } + } + + foreach(string file: get_dir("/tf-*.c")) + { + string err; + + msg("Running Test %s...\n", file[0..<3]); + + if((err = catch(load_object(file[0..<3]);nolog))) + { + msg(" Success.\n"); + } + else + { + errors++; + msg(" FAILURE! (No error occurred.)\n"); + } + } + + shutdown(errors && 1); +} + +string *epilog(int eflag) +{ + run_test(); + return 0; +} Index: trunk.switch/test/t-language/sys =================================================================== --- trunk.switch/test/t-language/sys (Revision 0) +++ trunk.switch/test/t-language/sys (Revision 0) @@ -0,0 +1 @@ +link ../sys \ Kein Zeilenvorschub am Ende der Datei Eigenschaftsänderungen: trunk.switch/test/t-language/sys ___________________________________________________________________ Name: svn:special + * Index: trunk.switch/src/version.sh =================================================================== --- trunk.switch/src/version.sh (Revision 2438) +++ trunk.switch/src/version.sh (Arbeitskopie) @@ -17,7 +17,7 @@ # A timestamp, to be used by bumpversion and other scripts. # It can be used, for example, to 'touch' this file on every build, thus # forcing revision control systems to add it on every checkin automatically. -version_stamp="2008-08-10 19:19:46" +version_stamp="Sa 6. Dez 18:32:40 CET 2008" # The version number information version_micro=717 Index: trunk.switch/src/prolang.y =================================================================== --- trunk.switch/src/prolang.y (Revision 2438) +++ trunk.switch/src/prolang.y (Arbeitskopie) @@ -6910,9 +6910,11 @@ /* Blocks and simple statements. */ -block: - '{' +block: '{' statements_block '}' + +statements_block: + { enter_block_scope(); } statements @@ -6933,14 +6935,12 @@ = (char)(scope->num_locals - scope->num_cleared); } } + + leave_block_scope(MY_FALSE); } +; /* block_statements */ - '}' - { leave_block_scope(MY_FALSE); } -; /* block */ - - statements: /* empty */ | statements local_name_list ';' @@ -7034,7 +7034,7 @@ } | error ';' /* Synchronisation point */ - | cond | while | do | for | foreach | switch | case | default + | cond | while | do | for | foreach | switch | return ';' | block | /* empty */ ';' @@ -8097,8 +8097,12 @@ if (current_continue_address) current_continue_address += SWITCH_DEPTH_UNIT; } + + '{' - block + switch_block + + '}' { %line @@ -8141,6 +8145,19 @@ } ; /* switch */ + +switch_block: + switch_block switch_statements + | switch_statements +; /* switch_block */ + + +switch_statements: switch_label statements_block ; + + +switch_label: case | default ; + + case: L_CASE case_label ':' { %line turn-parse-errors-into-assertions.patch (1,722 bytes)
commit 3ec662620b3476a496ccdb9b093532748ad2afb4 Author: Bertram Felgenhauer <int-e@gmx.de> Date: Wed Dec 10 16:08:26 2008 +0100 Turn parse errors into assertions. diff --git a/src/prolang.y b/src/prolang.y index 54a74d3..edf9bfa 100644 --- a/src/prolang.y +++ b/src/prolang.y @@ -87,6 +87,7 @@ #include <ctype.h> #include <stdio.h> #include <stdarg.h> +#include <assert.h> #include "prolang.h" @@ -8166,11 +8167,7 @@ case: L_CASE case_label ':' */ case_list_entry_t *temp; - if ( !( current_break_address & CASE_LABELS_ENABLED ) ) - { - yyerror("Case outside switch"); - break; - } + assert(current_break_address & CASE_LABELS_ENABLED); /* Get and fill in a new case entry structure */ if ( !(temp = new_case_entry()) ) @@ -8199,11 +8196,7 @@ case: L_CASE case_label ':' if ( !$2.numeric || !$4.numeric ) yyerror("String case labels not allowed as range bounds"); - if ( !( current_break_address & CASE_LABELS_ENABLED ) ) - { - yyerror("Case range outside switch"); - break; - } + assert(current_break_address & CASE_LABELS_ENABLED); /* A range like "case 4..2" is illegal, * a range like "case 4..4" counts as simple "case 4". @@ -8283,10 +8276,7 @@ default: * for the current switch. */ - if ( !( current_break_address & CASE_LABELS_ENABLED ) ) { - yyerror("Default outside switch"); - break; - } + assert(current_break_address & CASE_LABELS_ENABLED); if (case_state.default_addr) yyerror("Duplicate default"); | ||||
|
I uploaded a patch that changes the language parser, so that it explicitly expects case/default statements in the switch block and puts the non-case statements afterwards into their own block scope. |
|
Looks good to me. I've attached an additional patch that turns some parse errors that can't happen anymore into assertions. There's a slight degradation in the error message for "switch (0) { }" -- now it's just a "syntax error" while previously the message was "switch without case not supported" (because it parsed and failed later, when storing the case labels), but I think we can live with that. |
|
Applied both patches as r2454. |
Date Modified | Username | Field | Change |
---|---|---|---|
2008-12-03 02:24 | _xtian_ | New Issue | |
2008-12-04 07:16 | Gnomi | Status | new => assigned |
2008-12-04 07:16 | Gnomi | Assigned To | => Gnomi |
2008-12-06 11:48 | Gnomi | File Added: bug584.diff | |
2008-12-06 11:53 | Gnomi | Note Added: 0000810 | |
2008-12-10 09:10 | fufu | File Added: 0000548-turn-parse-errors-into-assertions.patch | |
2008-12-10 09:26 | fufu | Note Added: 0000812 | |
2008-12-23 18:56 | Gnomi | Project | LDMud => LDMud 3.3 |
2008-12-23 18:57 | Gnomi | Status | assigned => resolved |
2008-12-23 18:57 | Gnomi | Fixed in Version | => 3.3.718 |
2008-12-23 18:57 | Gnomi | Resolution | open => fixed |
2008-12-23 18:57 | Gnomi | Note Added: 0000828 | |
2010-11-16 09:42 | Gnomi | Source_changeset_attached | => ldmud.git master 77225dc8 |
2018-01-29 18:59 | Gnomi | Source_changeset_attached | => ldmud.git master 77225dc8 |
2018-01-29 21:57 | Gnomi | Source_changeset_attached | => ldmud.git master 77225dc8 |