PR fortran/46152 -- fix namespace pollution in type-spec matching

Hi Steve,

> See the PR for a thorough discussion of the problem and
> the fix.  The attached patch has been tested on i686-*-freebsd
> without regression.  I plan to commit this within the next
> 24 hours.

I think your patch is basically ok, so thanks for your work!

Just one little nit: I still don't quite like the fact that you simply
remove the one line in allocate_derived_1 where the error message has
changed.

What do you think about adding the following hunk in match.c to
improve the error message for this case:


@@ -2874,7 +2861,15 @@ gfc_match_allocate (void)
   if (m == MATCH_ERROR)
     goto cleanup;
   else if (m == MATCH_NO)
-    ts.type = BT_UNKNOWN;
+    {
+      char name[GFC_MAX_SYMBOL_LEN + 1];
+      if (gfc_match ("%n :: ", name) == MATCH_YES)
+       {
+         gfc_error ("Error in type-spec at %L", &old_locus);
+         goto cleanup;
+       }
+      ts.type = BT_UNKNOWN;
+    }
   else
     {
       if (gfc_match (" :: ") == MATCH_YES)


so that the test case becomes:



I think this would be much better than complaining about something
being wrong with the allocate-object, since the colons indicate that
the (optional) type-spec is present, so we might as well make use of
this information.

With this change the patch is ok from my side.

Cheers,
Janus



> 2010-10-30  Steven G. Kargl  <kargl@gcc.gnu.org>
>
>        PR fortran/46152
>        * gfortran.dg/select_type_11.f03: Update dg-error phrase.
>        * gfortran.dg/allocate_with_typespec_4.f90: New test.
>        * gfortran.dg/allocate_with_typespec_1.f90: New test.
>        * gfortran.dg/allocate_with_typespec_2.f: New test.
>        * gfortran.dg/allocate_with_typespec_3.f90: New test.
>        * gfortran.dg/allocate_derived_1.f90: Delete an obselescent test.
>        * gfortran.dg/select_type_1.f03: Update dg-error phrase.
>
> 2010-10-30  Steven G. Kargl  <kargl@gcc.gnu.org>
>
>        PR fortran/46152
>        * fortran/match.c (match_derived_type_spec): Reoplace gfc_match_symbol
>        with a gfc_find_symbol to prevent namespace pollution.  Remove dead
>        code.
>        (match_type_spec): Remove parsing of '::'.  Collapse character
>        kind checking to one location.
>        (gfc_match_allocate): Use correct locus in error message.
>
> --
> Steve
>

On Sun, Oct 31, 2010 at 11:54:47AM +0100, Janus Weil wrote:
> 
> What do you think about adding the following hunk in match.c to
> improve the error message for this case:
> 
> 
> @@ -2874,7 +2861,15 @@ gfc_match_allocate (void)
>    if (m == MATCH_ERROR)
>      goto cleanup;
>    else if (m == MATCH_NO)
> -    ts.type = BT_UNKNOWN;
> +    {
> +      char name[GFC_MAX_SYMBOL_LEN + 1];
> +      if (gfc_match ("%n :: ", name) == MATCH_YES)
> +       {
> +         gfc_error ("Error in type-spec at %L", &old_locus);
> +         goto cleanup;
> +       }
> +      ts.type = BT_UNKNOWN;
> +    }
>    else
>      {
>        if (gfc_match (" :: ") == MATCH_YES)
> 
> 
> so that the test case becomes:
> 
> 
> Index: gcc/testsuite/gfortran.dg/allocate_derived_1.f90
> ===================================================================
> --- gcc/testsuite/gfortran.dg/allocate_derived_1.f90    (revision 166088)
> +++ gcc/testsuite/gfortran.dg/allocate_derived_1.f90    (working copy)
> @@ -32,7 +32,7 @@
>   allocate(t1 :: x(2))
>   allocate(t2 :: x(3))
>   allocate(t3 :: x(4))
> - allocate(tx :: x(5))  ! { dg-error "is not an accessible derived type" }
> + allocate(tx :: x(5))  ! { dg-error "Error in type-spec" }
>   allocate(u0 :: x(6))  ! { dg-error "may not be ABSTRACT" }
>   allocate(v1 :: x(7))  ! { dg-error "is type incompatible with typespec" }
> 
> 
> I think this would be much better than complaining about something
> being wrong with the allocate-object, since the colons indicate that
> the (optional) type-spec is present, so we might as well make use of
> this information.
> 
> With this change the patch is ok from my side.
> 

Thanks.  Your suggestion is exactly what was needed!
I could not come up with what I considered a clean 
solution for 

module a
   type rael
     integer i
   end type rael
   type b
     real x
   end type b
end module a

program c
   use a, only : b
   real, allocatable :: x(:)
   allocate(rael :: x(1))    ! inaccessible type or a typographical error?
end program c

On 10/31/2010 07:37 AM, Steve Kargl wrote:
> On Sun, Oct 31, 2010 at 11:54:47AM +0100, Janus Weil wrote:
>>
>> What do you think about adding the following hunk in match.c to
>> improve the error message for this case:
>>
>>
>> @@ -2874,7 +2861,15 @@ gfc_match_allocate (void)
>>     if (m == MATCH_ERROR)
>>       goto cleanup;
>>     else if (m == MATCH_NO)
>> -    ts.type = BT_UNKNOWN;
>> +    {
>> +      char name[GFC_MAX_SYMBOL_LEN + 1];
>> +      if (gfc_match ("%n :: ", name) == MATCH_YES)
>> +       {
>> +         gfc_error ("Error in type-spec at %L",&old_locus);
>> +         goto cleanup;
>> +       }
>> +      ts.type = BT_UNKNOWN;
>> +    }
>>     else
>>       {
>>         if (gfc_match (" :: ") == MATCH_YES)
>>
>>
>> so that the test case becomes:
>>
>>
>> Index: gcc/testsuite/gfortran.dg/allocate_derived_1.f90
>> ===================================================================
>> --- gcc/testsuite/gfortran.dg/allocate_derived_1.f90    (revision 166088)
>> +++ gcc/testsuite/gfortran.dg/allocate_derived_1.f90    (working copy)
>> @@ -32,7 +32,7 @@
>>    allocate(t1 :: x(2))
>>    allocate(t2 :: x(3))
>>    allocate(t3 :: x(4))
>> - allocate(tx :: x(5))  ! { dg-error "is not an accessible derived type" }
>> + allocate(tx :: x(5))  ! { dg-error "Error in type-spec" }
>>    allocate(u0 :: x(6))  ! { dg-error "may not be ABSTRACT" }
>>    allocate(v1 :: x(7))  ! { dg-error "is type incompatible with typespec" }
>>
>>
>> I think this would be much better than complaining about something
>> being wrong with the allocate-object, since the colons indicate that
>> the (optional) type-spec is present, so we might as well make use of
>> this information.
>>
>> With this change the patch is ok from my side.
>>
>
> Thanks.  Your suggestion is exactly what was needed!
> I could not come up with what I considered a clean
> solution for
>
> module a
>     type rael
>       integer i
>     end type rael
>     type b
>       real x
>     end type b
> end module a
>
> program c
>     use a, only : b
>     real, allocatable :: x(:)
>     allocate(rael :: x(1))    ! inaccessible type or a typographical error?
> end program c
>

You are approved to commit now if regression testing passes.  No need to wait.

Jerry

On Sun, Oct 31, 2010 at 09:37:55AM -0700, Jerry DeLisle wrote:
> 
> You are approved to commit now if regression testing passes.  No need to 
> wait.
> 

Committed with Janus's suggested change (argh,
just realized I forgot to include Janus in the
ChangeLog entry :(.

svn-commit.tmp: 20 lines, 842 characters.
Sending        gcc/fortran/ChangeLog
Sending        gcc/fortran/match.c
Sending        gcc/testsuite/ChangeLog
Sending        gcc/testsuite/gfortran.dg/allocate_derived_1.f90
Adding         gcc/testsuite/gfortran.dg/allocate_with_typespec_1.f90
Adding         gcc/testsuite/gfortran.dg/allocate_with_typespec_2.f
Adding         gcc/testsuite/gfortran.dg/allocate_with_typespec_3.f90
Adding         gcc/testsuite/gfortran.dg/allocate_with_typespec_4.f90
Sending        gcc/testsuite/gfortran.dg/select_type_1.f03
Sending        gcc/testsuite/gfortran.dg/select_type_11.f03
Transmitting file data ..........
Committed revision 166140.