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 |