[patch, fortran] PR34325 Wrong error message for syntax error

[patch, fortran] PR34325 Wrong error message for syntax error

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

Seasons Greetings,

This patch adds a new function to check for missing parenthesis in a statement. 
  I tried to use this up front in decode_statement but it regresses several test 
cases that are finding the error by other means.

You have to be careful where you use this as well since some matchers get called 
several times.  So you will see in gfc_match_if where I use this that I put the 
check after the match for the IF has occurred.  This requires some back 
tracking, but it is fairly painless..

The patch just fixes this one bug in this one place in gfc_match_if.  The new 
function can be used selectively in other places as we discover them. 
(Especially after this email and people start looking for them :) )

I attempted to make sure we ignore quoted strings.  See the test case.

Regression tested on x86-64.

OK to commit?

Jerry

2007-12-14  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/34325
	* match.h: New function declaration.
	* match.c (gfc_match_parens): New function to look for mismatched
	parenthesis. (gfc_match_if): Use new function to catch missing '('.

[-- Attachment #2: pr34325.diff --]
[-- Type: text/x-patch, Size: 2814 bytes --]

Index: match.c
===================================================================
--- match.c	(revision 130927)
+++ match.c	(working copy)
@@ -104,6 +104,68 @@ gfc_op2string (gfc_intrinsic_op op)
 
 /******************** Generic matching subroutines ************************/
 
+/* This function scans the current statement counting the opened and closed
+   parenthesis to make sure they are balanced.  */
+
+match
+gfc_match_parens (void)
+{
+  locus old_loc, where;
+  int c, count, instring;
+  char quote;
+
+  old_loc = gfc_current_locus;
+  count = 0;
+  instring = 0;
+  quote = ' ';
+
+  for (;;)
+    {
+      c = gfc_next_char_literal (instring);
+      if (c == '\n')
+	break;
+      if (quote == ' ' && ((c == '\'') || (c == '"')))
+	{
+	  quote = (char) c;
+          instring = 1;
+	  continue;
+	}
+      if (quote != ' ' && c == quote)
+	{
+	  quote = ' ';
+          instring = 0;
+	  continue;
+	}
+
+      if (c == '(' && quote == ' ')
+	{
+	  count++;
+	  where = gfc_current_locus;
+	}
+      if (c == ')' && quote == ' ')
+	{
+	  count--;
+	  where = gfc_current_locus;
+	}
+    }
+
+  gfc_current_locus = old_loc;
+
+  if (count > 0)
+    {
+      gfc_error ("Missing ')' in statement at %L", &where);
+      return MATCH_ERROR;
+    }
+  if (count < 0)
+    {
+      gfc_error ("Missing '(' in statement at %L", &where);
+      return MATCH_ERROR;
+    }
+
+  return MATCH_YES;
+}
+
+
 /* See if the next character is a special character that has
    escaped by a \ via the -fbackslash option.  */
 
@@ -1321,7 +1383,7 @@ gfc_match_if (gfc_statement *if_type)
 {
   gfc_expr *expr;
   gfc_st_label *l1, *l2, *l3;
-  locus old_loc;
+  locus old_loc, old_loc2;
   gfc_code *p;
   match m, n;
 
@@ -1335,6 +1397,14 @@ gfc_match_if (gfc_statement *if_type)
   if (m != MATCH_YES)
     return m;
 
+  old_loc2 = gfc_current_locus;
+  gfc_current_locus = old_loc;
+  
+  if (gfc_match_parens () == MATCH_ERROR)
+    return MATCH_ERROR;
+
+  gfc_current_locus = old_loc2;
+
   if (gfc_match_char (')') != MATCH_YES)
     {
       gfc_error ("Syntax error in IF-expression at %C");
@@ -1386,7 +1456,7 @@ gfc_match_if (gfc_statement *if_type)
 
   if (n == MATCH_YES)
     {
-      gfc_error ("Block label is not appropriate IF statement at %C");
+      gfc_error ("Block label is not appropriate for IF statement at %C");
       gfc_free_expr (expr);
       return MATCH_ERROR;
     }
Index: match.h
===================================================================
--- match.h	(revision 130927)
+++ match.h	(working copy)
@@ -54,6 +54,7 @@ match gfc_match_intrinsic_op (gfc_intrin
 match gfc_match_char (char);
 match gfc_match (const char *, ...);
 match gfc_match_iterator (gfc_iterator *, int);
+match gfc_match_parens (void);
 
 /* Statement matchers.  */
 match gfc_match_program (void);

[-- Attachment #3: missing_parens_1.f90 --]
[-- Type: text/x-fortran, Size: 464 bytes --]

! { dg-do compile }
! PR34325 Wrong error message for syntax error
program aa
implicit none
real(kind=8)::r1=0
character(25) :: a
a = 'I am not a )))))'')''.'
if ((((((a /= "I am not a )))))')'.")))))) call abort
if ((((((a /= 'I am not a )))))'')''.')))))) call abort
a = "I am not a )))))"")""."
if ((((((a /= "I am not a )))))"")"".")))))) call abort
if (((3*r1)**2)>= 0) a = "good"
if ((3*r1)**2)>= 0) a = "bad" ! { dg-error "Missing '\\(' in statement" }
end

Re: [patch, fortran] PR34325 Wrong error message for syntax error

[Tobias Burnus]

Jerry DeLisle wrote:
> Regression tested on x86-64.
> OK to commit?

+       {
+         quote = (char) c;
+          instring = 1;
+         continue;


Indentation.

Otherwise it looks OK.

Tobias

Re: [patch, fortran] PR34325 Wrong error message for syntax error

[FX]

> 2008-05-04  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
>
> 	PR fortran/34325
> 	* decl.c (match_attr_spec): Check for matching pairs of parenthesis.
> 	* expr.c (gfc_specification_expr): Supplement the error message  
> with the
> 	type that was found.
> 	* resolve.c (gfc_resolve_index): Likewise.
> 	* match.c (gfc_match_parens): Clarify error message with "at or  
> before".
> 	(gfc_match_do): Check for matching pairs of parenthesis.

OK to commit. I think that generally these syntax-error and  
diagnostic patches are rather hard to review in depth, as there are  
really tons of ways they could do things we can't even imagine on  
other invalid-code cases, so I tend to only review them for style,  
wording and obvious mistakes; I hope it's OK.


FX

-- 
François-Xavier Coudert
http://www.homepages.ucl.ac.uk/~uccafco/

[patch, fortran] PR34325 Wrong error message for syntax error

[Jerry DeLisle]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

This patch provides some additional checks for balanced parenthesis in a couple 
of strategic locations.  This will not catch all the possibilities, but is a 
reasonable enhancement.

I noticed in some cases of double parens for array references such as:

	r3((2,2)) = 4.3

gfortran correctly finds the error stating that the index must be integer.  It 
was not real clear that in the above (2,2) is a COMPLEX.  Ifort and Sun F95 both 
add additional info that a COMPLEX was found.  I supplemented the gfortran error 
to do similarly.

Regression tested on x86-64.  Revised test case included in patch.

OK for trunk?

Regards,

Jerry

2008-05-04  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/34325
	* decl.c (match_attr_spec): Check for matching pairs of parenthesis.
	* expr.c (gfc_specification_expr): Supplement the error message with the
	type that was found.
	* resolve.c (gfc_resolve_index): Likewise.
	* match.c (gfc_match_parens): Clarify error message with "at or before".
	(gfc_match_do): Check for matching pairs of parenthesis.



[-- Attachment #2: pr34325.diff --]
[-- Type: text/x-patch, Size: 3516 bytes --]

Index: fortran/decl.c
===================================================================
--- fortran/decl.c	(revision 134899)
+++ fortran/decl.c	(working copy)
@@ -2931,6 +2931,13 @@ match_attr_spec (void)
 	  goto cleanup;
 	}
 
+      /* Check to make sure any parens are paired up correctly.  */
+      if (gfc_match_parens () == MATCH_ERROR)
+	{
+	  m = MATCH_ERROR;
+	  goto cleanup;
+	}
+
       seen[d]++;
       seen_at[d] = gfc_current_locus;
 
Index: fortran/expr.c
===================================================================
--- fortran/expr.c	(revision 134899)
+++ fortran/expr.c	(working copy)
@@ -2571,7 +2571,8 @@ gfc_specification_expr (gfc_expr *e)
 
   if (e->ts.type != BT_INTEGER)
     {
-      gfc_error ("Expression at %L must be of INTEGER type", &e->where);
+      gfc_error ("Expression at %L must be of INTEGER type, found %s",
+		 &e->where, gfc_basic_typename (e->ts.type));
       return FAILURE;
     }
 
Index: fortran/match.c
===================================================================
--- fortran/match.c	(revision 134899)
+++ fortran/match.c	(working copy)
@@ -153,12 +153,12 @@ gfc_match_parens (void)
 
   if (count > 0)
     {
-      gfc_error ("Missing ')' in statement before %L", &where);
+      gfc_error ("Missing ')' in statement at or before %L", &where);
       return MATCH_ERROR;
     }
   if (count < 0)
     {
-      gfc_error ("Missing '(' in statement before %L", &where);
+      gfc_error ("Missing '(' in statement at or before %L", &where);
       return MATCH_ERROR;
     }
 
@@ -525,7 +525,6 @@ gfc_match_name (char *buffer)
       return MATCH_ERROR;
     }
 
-
   buffer[i] = '\0';
   gfc_current_locus = old_loc;
 
@@ -1719,6 +1718,11 @@ gfc_match_do (void)
   if (gfc_match_char (',') != MATCH_YES && gfc_match ("% ") != MATCH_YES)
     return MATCH_NO;
 
+  /* Check for balanced parens.  */
+  
+  if (gfc_match_parens () == MATCH_ERROR)
+    return MATCH_ERROR;
+
   /* See if we have a DO WHILE.  */
   if (gfc_match (" while ( %e )%t", &iter.end) == MATCH_YES)
     {
Index: fortran/resolve.c
===================================================================
--- fortran/resolve.c	(revision 134899)
+++ fortran/resolve.c	(working copy)
@@ -3510,8 +3510,8 @@ gfc_resolve_index (gfc_expr *index, int 
 
   if (index->ts.type != BT_INTEGER && index->ts.type != BT_REAL)
     {
-      gfc_error ("Array index at %L must be of INTEGER type",
-		 &index->where);
+      gfc_error ("Array index at %L must be of INTEGER type, found %s",
+		 &index->where, gfc_basic_typename (index->ts.type));
       return FAILURE;
     }
 
Index: testsuite/gfortran.dg/missing_parens_1.f90
===================================================================
--- testsuite/gfortran.dg/missing_parens_1.f90	(revision 134845)
+++ testsuite/gfortran.dg/missing_parens_1.f90	(working copy)
@@ -3,6 +3,8 @@
 program aa
 implicit none
 real(kind=8)::r1=0
+real(kind=8),dimension((1)::r2 ! { dg-error "Missing '\\)' in statement" }
+real(kind=8),dimension(3,3)::r3
 character(25) :: a
 a = 'I am not a )))))'')''.'
 if ((((((a /= "I am not a )))))')'.")))))) call abort
@@ -11,4 +13,7 @@ a = "I am not a )))))"")""."
 if ((((((a /= "I am not a )))))"")"".")))))) call abort
 if (((3*r1)**2)>= 0) a = "good"
 if ((3*r1)**2)>= 0) a = "bad" ! { dg-error "Missing '\\(' in statement" }
+r3((2,2)) = 4.3 ! { dg-error "found COMPLEX" }
+do while ((.true.) ! { dg-error "Missing '\\)' in statement" }
+do while (.true. ! { dg-error "Missing '\\)' in statement" }
 end