View Issue Details

IDProjectCategoryView StatusLast Update
0000584LDMud 3.3LPC Compiler/Preprocessorpublic2018-01-29 22:57
Reporter_xtian_ Assigned ToGnomi  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platform64bitOSdebianOS Versionlenny
Product Version3.3 
Fixed in Version3.3.718 
Summary0000584: variable definition in outer switch block
DescriptionIt 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 Reproducemixed test()
{
  int i=1;

  switch(i)
  {
    mixed mx=2; // either: this definition shouldnt be allowed here

    case 1:
    // ...
      return mx; // or: this yield 0
  }
}
TagsNo tags attached.

Activities

2008-12-06 12:48

 

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
bug584.diff (5,838 bytes)   

Gnomi

2008-12-06 12:53

manager   ~0000810

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.

2008-12-10 10:10

 

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");

fufu

2008-12-10 10:26

manager   ~0000812

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.

Gnomi

2008-12-23 19:57

manager   ~0000828

Applied both patches as r2454.

Issue History

Date Modified Username Field Change
2008-12-03 03:24 _xtian_ New Issue
2008-12-04 08:16 Gnomi Status new => assigned
2008-12-04 08:16 Gnomi Assigned To => Gnomi
2008-12-06 12:48 Gnomi File Added: bug584.diff
2008-12-06 12:53 Gnomi Note Added: 0000810
2008-12-10 10:10 fufu File Added: 0000548-turn-parse-errors-into-assertions.patch
2008-12-10 10:26 fufu Note Added: 0000812
2008-12-23 19:56 Gnomi Project LDMud => LDMud 3.3
2008-12-23 19:57 Gnomi Status assigned => resolved
2008-12-23 19:57 Gnomi Fixed in Version => 3.3.718
2008-12-23 19:57 Gnomi Resolution open => fixed
2008-12-23 19:57 Gnomi Note Added: 0000828
2010-11-16 10:42 Gnomi Source_changeset_attached => ldmud.git master 77225dc8
2018-01-29 19:59 Gnomi Source_changeset_attached => ldmud.git master 77225dc8
2018-01-29 22:57 Gnomi Source_changeset_attached => ldmud.git master 77225dc8