public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
* Ping: New suggested patch for pr 62242 & pr 52332
@ 2015-09-26  9:18 Louis Krupp
  2015-09-28  7:55 ` Paul Richard Thomas
  0 siblings, 1 reply; 5+ messages in thread
From: Louis Krupp @ 2015-09-26  9:18 UTC (permalink / raw)
  To: fortran

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

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog    (revision 227895)
+++ gcc/fortran/ChangeLog    (working copy)
@@ -1,3 +1,15 @@
+2015-09-18 Louis Krupp <louis.krupp@zoho.com>
+
+    PR fortran/62242
+    PR fortran/52332
+    * trans-array.c
+    (store_backend_decl): Create new gfc_charlen instance if requested
+    (get_array_ctor_all_strlen): Call store_backend_decl requesting
+    new gfc_charlen
+    (trans_array_constructor): Call store_backend_decl requesting
+    new gfc_charlen if get_array_ctor_strlen was called
+    (gfc_add_loop_ss_code): Don't try to convert non-constant length
+
 2015-09-17 Paul Thomas <pault@gcc.gnu.org>
 
     PR fortran/52846
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog    (revision 227895)
+++ gcc/testsuite/ChangeLog    (working copy)
@@ -1,3 +1,10 @@
+2015-09-18 Louis Krupp <louis.krupp@zoho.com>
+
+    PR fortran/62242 fortran/52332
+    * gfortran.dg/string_array_constructor_1.f90: New.
+    * gfortran.dg/string_array_constructor_2.f90: New.
+    * gfortran.dg/string_array_constructor_3.f90: New.
+
 2015-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de>
 
     PR sanitizer/64078
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c    (revision 227895)
+++ gcc/fortran/trans-array.c    (working copy)
@@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p
 }
 
 
+/* The array constructor code can create a string length with an operand
+ in the form of a temporary variable. This variable will retain its
+ context (current_function_decl). If we store this length tree in a
+ gfc_charlen structure which is shared by a variable in another
+ context, the resulting gfc_charlen structure with a variable in a
+ different context, we could trip the assertion in expand_expr_real_1
+ when it sees that a variable has been created in one context and
+ referenced in another:
+
+ if (exp)
+     context = decl_function_context (exp);
+ gcc_assert (!exp
+         || SCOPE_FILE_SCOPE_P (context)
+         || context == current_function_decl
+         || TREE_STATIC (exp)
+         || DECL_EXTERNAL (exp)
+         // ??? C++ creates functions that are not TREE_STATIC.
+         || TREE_CODE (exp) == FUNCTION_DECL);
+
+ If this might be the case, we create a new gfc_charlen structure and
+ link it into the current namespace. */
+
+static void
+store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl)
+{
+ if (force_new_cl)
+ {
+ gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp);
+ *clp = new_cl;
+ }
+ (*clp)->backend_decl = len;
+}
+
 /* A catch-all to obtain the string length for anything that is not
 a substring of non-constant length, a constant, array or variable. */
 
@@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc
 gfc_add_block_to_block (block, &se.pre);
 gfc_add_block_to_block (block, &se.post);
 
- e->ts.u.cl->backend_decl = *len;
+ store_backend_decl (&e->ts.u.cl, *len, true);
 }
 }
 
@@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher
 if (expr->ts.type == BT_CHARACTER)
 {
 bool const_string;
+ bool force_new_cl = false;
 
 /* get_array_ctor_strlen walks the elements of the constructor, if a
      typespec was given, we already know the string length and want the one
@@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher
      gfc_add_block_to_block (&outer_loop->post, &length_se.post);
     }
 else
-    const_string = get_array_ctor_strlen (&outer_loop->pre, c,
-                     &ss_info->string_length);
+    {
+     const_string = get_array_ctor_strlen (&outer_loop->pre, c,
+                        &ss_info->string_length);
+     force_new_cl = true;
+    }
 
 /* Complex character array constructors should have been taken care of
      and not end up here. */
 gcc_assert (ss_info->string_length);
 
- expr->ts.u.cl->backend_decl = ss_info->string_length;
+ store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl);
 
 type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
 if (const_string)
@@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss
      if (expr->ts.type == BT_CHARACTER
      && ss_info->string_length == NULL
      && expr->ts.u.cl
-     && expr->ts.u.cl->length)
+     && expr->ts.u.cl->length
+     && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
      {
      gfc_init_se (&se, NULL);
      gfc_conv_expr_type (&se, expr->ts.u.cl->length,

[-- Attachment #2: string_array_constructor_1.f90 --]
[-- Type: application/octet-stream, Size: 774 bytes --]

! { dg-do compile }
! PR 62242
! PR 52332
! Array constructor with an array element whose value is a
! character function that is described in an interface block and which
! has an assumed-length result
module gfbug
    implicit none
    INTERFACE
      function UpperCase(string) result(upper) 
          character(*), intent(IN) :: string
          character(LEN(string)) :: upper
      end function
      function f2(string) result(upper) 
          character(*), intent(IN) :: string
          character(5) :: upper
      end function
    END INTERFACE
contains
    subroutine s1
        character(5) c
        character(5), dimension(1) :: ca
        ca = (/f2(c)/)  ! This compiles
        ca = (/Uppercase(c)/) ! This gets an ICE
    end subroutine
end module gfbug


[-- Attachment #3: string_array_constructor_2.f90 --]
[-- Type: application/octet-stream, Size: 1283 bytes --]

! { dg-do run }
! PR 62242
! PR 52332
! Array constructor with an array element whose value is a
! character function that is described in an interface block and which
! has an assumed-length result
module gfbug
    implicit none
    INTERFACE
      function UpperCase(string) result(upper) 
          character(*), intent(IN) :: string
          character(LEN(string)) :: upper
      end function
      function f2(string) result(upper) 
          character(*), intent(IN) :: string
          character(5) :: upper
      end function
    END INTERFACE
contains
    subroutine s1
        character(5) c
        character(5), dimension(1) :: ca
        character(5), dimension(1) :: cb
        c = "12345"
        ca = (/f2(c)/) ! This works
        !print *, ca(1)
        cb = (/Uppercase(c)/) ! This gets an ICE
        if (ca(1) .ne. cb(1)) then
            call abort()
        end if
        !print *, ca(1)
    end subroutine
end module gfbug

function UpperCase(string) result(upper) 
    character(*), intent(IN) :: string
    character(LEN(string)) :: upper
    upper = string
end function
function f2(string) result(upper) 
    character(*), intent(IN) :: string
    character(5) :: upper
    upper = string
end function

program main
    use gfbug
    call s1
end program

[-- Attachment #4: string_array_constructor_3.f90 --]
[-- Type: application/octet-stream, Size: 625 bytes --]

! { dg-do compile }
! PR 62242
! PR 52332
! A subprogram calling an array constructor with an array element whose
! value is the result of calling a character function with both an
! assumed-length argument and an assumed-length result
module gfbug
    implicit none
contains
    function inner(inner_str) result(upper)
        character(*), intent(IN) :: inner_str
        character(LEN(inner_str)) :: upper

        upper = '123'
    end function

    subroutine outer(outer_str)
        character(*), intent(IN) :: outer_str
        character(5) :: z(1)

        z = [inner(outer_str)]
    end subroutine
end module gfbug

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Ping: New suggested patch for pr 62242 & pr 52332
  2015-09-26  9:18 Ping: New suggested patch for pr 62242 & pr 52332 Louis Krupp
@ 2015-09-28  7:55 ` Paul Richard Thomas
  2015-09-29  7:36   ` Louis Krupp
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Richard Thomas @ 2015-09-28  7:55 UTC (permalink / raw)
  To: Louis Krupp; +Cc: fortran

Hi Louis,

Could you please send the patch as an attachment - in your message,
much of the lhs whitespace information has been lost. Fundamentally,
the patch looks OK. Since they pertain to the same PRs, I would
consider combining the first and third testcases, either by throwing
the procedures into one module of by renaming the modules; eg. gfbug1
and gfbug2.

Cheers

Paul

On 25 September 2015 at 18:49, Louis Krupp <louis.krupp@zoho.com> wrote:
> Index: gcc/fortran/ChangeLog
> ===================================================================
> --- gcc/fortran/ChangeLog    (revision 227895)
> +++ gcc/fortran/ChangeLog    (working copy)
> @@ -1,3 +1,15 @@
> +2015-09-18 Louis Krupp <louis.krupp@zoho.com>
> +
> +    PR fortran/62242
> +    PR fortran/52332
> +    * trans-array.c
> +    (store_backend_decl): Create new gfc_charlen instance if requested
> +    (get_array_ctor_all_strlen): Call store_backend_decl requesting
> +    new gfc_charlen
> +    (trans_array_constructor): Call store_backend_decl requesting
> +    new gfc_charlen if get_array_ctor_strlen was called
> +    (gfc_add_loop_ss_code): Don't try to convert non-constant length
> +
>  2015-09-17 Paul Thomas <pault@gcc.gnu.org>
>
>      PR fortran/52846
> Index: gcc/testsuite/ChangeLog
> ===================================================================
> --- gcc/testsuite/ChangeLog    (revision 227895)
> +++ gcc/testsuite/ChangeLog    (working copy)
> @@ -1,3 +1,10 @@
> +2015-09-18 Louis Krupp <louis.krupp@zoho.com>
> +
> +    PR fortran/62242 fortran/52332
> +    * gfortran.dg/string_array_constructor_1.f90: New.
> +    * gfortran.dg/string_array_constructor_2.f90: New.
> +    * gfortran.dg/string_array_constructor_3.f90: New.
> +
>  2015-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de>
>
>      PR sanitizer/64078
> Index: gcc/fortran/trans-array.c
> ===================================================================
> --- gcc/fortran/trans-array.c    (revision 227895)
> +++ gcc/fortran/trans-array.c    (working copy)
> @@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p
>  }
>
>
> +/* The array constructor code can create a string length with an operand
> + in the form of a temporary variable. This variable will retain its
> + context (current_function_decl). If we store this length tree in a
> + gfc_charlen structure which is shared by a variable in another
> + context, the resulting gfc_charlen structure with a variable in a
> + different context, we could trip the assertion in expand_expr_real_1
> + when it sees that a variable has been created in one context and
> + referenced in another:
> +
> + if (exp)
> +     context = decl_function_context (exp);
> + gcc_assert (!exp
> +         || SCOPE_FILE_SCOPE_P (context)
> +         || context == current_function_decl
> +         || TREE_STATIC (exp)
> +         || DECL_EXTERNAL (exp)
> +         // ??? C++ creates functions that are not TREE_STATIC.
> +         || TREE_CODE (exp) == FUNCTION_DECL);
> +
> + If this might be the case, we create a new gfc_charlen structure and
> + link it into the current namespace. */
> +
> +static void
> +store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl)
> +{
> + if (force_new_cl)
> + {
> + gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp);
> + *clp = new_cl;
> + }
> + (*clp)->backend_decl = len;
> +}
> +
>  /* A catch-all to obtain the string length for anything that is not
>  a substring of non-constant length, a constant, array or variable. */
>
> @@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc
>  gfc_add_block_to_block (block, &se.pre);
>  gfc_add_block_to_block (block, &se.post);
>
> - e->ts.u.cl->backend_decl = *len;
> + store_backend_decl (&e->ts.u.cl, *len, true);
>  }
>  }
>
> @@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher
>  if (expr->ts.type == BT_CHARACTER)
>  {
>  bool const_string;
> + bool force_new_cl = false;
>
>  /* get_array_ctor_strlen walks the elements of the constructor, if a
>       typespec was given, we already know the string length and want the one
> @@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher
>       gfc_add_block_to_block (&outer_loop->post, &length_se.post);
>      }
>  else
> -    const_string = get_array_ctor_strlen (&outer_loop->pre, c,
> -                     &ss_info->string_length);
> +    {
> +     const_string = get_array_ctor_strlen (&outer_loop->pre, c,
> +                        &ss_info->string_length);
> +     force_new_cl = true;
> +    }
>
>  /* Complex character array constructors should have been taken care of
>       and not end up here. */
>  gcc_assert (ss_info->string_length);
>
> - expr->ts.u.cl->backend_decl = ss_info->string_length;
> + store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl);
>
>  type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
>  if (const_string)
> @@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss
>       if (expr->ts.type == BT_CHARACTER
>       && ss_info->string_length == NULL
>       && expr->ts.u.cl
> -     && expr->ts.u.cl->length)
> +     && expr->ts.u.cl->length
> +     && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
>       {
>       gfc_init_se (&se, NULL);
>       gfc_conv_expr_type (&se, expr->ts.u.cl->length,



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Re: Ping: New suggested patch for pr 62242 & pr 52332
  2015-09-28  7:55 ` Paul Richard Thomas
@ 2015-09-29  7:36   ` Louis Krupp
  2015-10-01  9:10     ` Paul Richard Thomas
  0 siblings, 1 reply; 5+ messages in thread
From: Louis Krupp @ 2015-09-29  7:36 UTC (permalink / raw)
  To: Paul Richard Thomas; +Cc: fortran

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

Paul,

The patch is attached.  I last compiled and tested the code at 21:15 UTC on 28 September.

Would you like me to resend the test cases?

Louis

---- On Mon, 28 Sep 2015 00:55:10 -0700 Paul Richard Thomas  wrote ---- 
>Hi Louis, 
> 
>Could you please send the patch as an attachment - in your message, 
>much of the lhs whitespace information has been lost. Fundamentally, 
>the patch looks OK. Since they pertain to the same PRs, I would 
>consider combining the first and third testcases, either by throwing 
>the procedures into one module of by renaming the modules; eg. gfbug1 
>and gfbug2. 
> 
>Cheers 
> 
>Paul 
> 
>On 25 September 2015 at 18:49, Louis Krupp <louis.krupp@zoho.com> wrote: 
>> Index: gcc/fortran/ChangeLog 
>> =================================================================== 
>> --- gcc/fortran/ChangeLog (revision 227895) 
>> +++ gcc/fortran/ChangeLog (working copy) 
>> @@ -1,3 +1,15 @@ 
>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com> 
>> + 
>> + PR fortran/62242 
>> + PR fortran/52332 
>> + * trans-array.c 
>> + (store_backend_decl): Create new gfc_charlen instance if requested 
>> + (get_array_ctor_all_strlen): Call store_backend_decl requesting 
>> + new gfc_charlen 
>> + (trans_array_constructor): Call store_backend_decl requesting 
>> + new gfc_charlen if get_array_ctor_strlen was called 
>> + (gfc_add_loop_ss_code): Don't try to convert non-constant length 
>> + 
>> 2015-09-17 Paul Thomas <pault@gcc.gnu.org> 
>> 
>> PR fortran/52846 
>> Index: gcc/testsuite/ChangeLog 
>> =================================================================== 
>> --- gcc/testsuite/ChangeLog (revision 227895) 
>> +++ gcc/testsuite/ChangeLog (working copy) 
>> @@ -1,3 +1,10 @@ 
>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com> 
>> + 
>> + PR fortran/62242 fortran/52332 
>> + * gfortran.dg/string_array_constructor_1.f90: New. 
>> + * gfortran.dg/string_array_constructor_2.f90: New. 
>> + * gfortran.dg/string_array_constructor_3.f90: New. 
>> + 
>> 2015-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de> 
>> 
>> PR sanitizer/64078 
>> Index: gcc/fortran/trans-array.c 
>> =================================================================== 
>> --- gcc/fortran/trans-array.c (revision 227895) 
>> +++ gcc/fortran/trans-array.c (working copy) 
>> @@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p 
>> } 
>> 
>> 
>> +/* The array constructor code can create a string length with an operand 
>> + in the form of a temporary variable. This variable will retain its 
>> + context (current_function_decl). If we store this length tree in a 
>> + gfc_charlen structure which is shared by a variable in another 
>> + context, the resulting gfc_charlen structure with a variable in a 
>> + different context, we could trip the assertion in expand_expr_real_1 
>> + when it sees that a variable has been created in one context and 
>> + referenced in another: 
>> + 
>> + if (exp) 
>> + context = decl_function_context (exp); 
>> + gcc_assert (!exp 
>> + || SCOPE_FILE_SCOPE_P (context) 
>> + || context == current_function_decl 
>> + || TREE_STATIC (exp) 
>> + || DECL_EXTERNAL (exp) 
>> + // ??? C++ creates functions that are not TREE_STATIC. 
>> + || TREE_CODE (exp) == FUNCTION_DECL); 
>> + 
>> + If this might be the case, we create a new gfc_charlen structure and 
>> + link it into the current namespace. */ 
>> + 
>> +static void 
>> +store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl) 
>> +{ 
>> + if (force_new_cl) 
>> + { 
>> + gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp); 
>> + *clp = new_cl; 
>> + } 
>> + (*clp)->backend_decl = len; 
>> +} 
>> + 
>> /* A catch-all to obtain the string length for anything that is not 
>> a substring of non-constant length, a constant, array or variable. */ 
>> 
>> @@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc 
>> gfc_add_block_to_block (block, &se.pre); 
>> gfc_add_block_to_block (block, &se.post); 
>> 
>> - e->ts.u.cl->backend_decl = *len; 
>> + store_backend_decl (&e->ts.u.cl, *len, true); 
>> } 
>> } 
>> 
>> @@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher 
>> if (expr->ts.type == BT_CHARACTER) 
>> { 
>> bool const_string; 
>> + bool force_new_cl = false; 
>> 
>> /* get_array_ctor_strlen walks the elements of the constructor, if a 
>> typespec was given, we already know the string length and want the one 
>> @@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher 
>> gfc_add_block_to_block (&outer_loop->post, &length_se.post); 
>> } 
>> else 
>> - const_string = get_array_ctor_strlen (&outer_loop->pre, c, 
>> - &ss_info->string_length); 
>> + { 
>> + const_string = get_array_ctor_strlen (&outer_loop->pre, c, 
>> + &ss_info->string_length); 
>> + force_new_cl = true; 
>> + } 
>> 
>> /* Complex character array constructors should have been taken care of 
>> and not end up here. */ 
>> gcc_assert (ss_info->string_length); 
>> 
>> - expr->ts.u.cl->backend_decl = ss_info->string_length; 
>> + store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl); 
>> 
>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length); 
>> if (const_string) 
>> @@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss 
>> if (expr->ts.type == BT_CHARACTER 
>> && ss_info->string_length == NULL 
>> && expr->ts.u.cl 
>> - && expr->ts.u.cl->length) 
>> + && expr->ts.u.cl->length 
>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT) 
>> { 
>> gfc_init_se (&se, NULL); 
>> gfc_conv_expr_type (&se, expr->ts.u.cl->length, 
> 
> 
> 
>-- 
>Outside of a dog, a book is a man's best friend. Inside of a dog it's 
>too dark to read. 
> 
>Groucho Marx 
>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4728 bytes --]

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(revision 228220)
+++ gcc/fortran/ChangeLog	(working copy)
@@ -1,3 +1,15 @@
+2015-09-28  Louis Krupp <louis.krupp@zoho.com>
+
+	PR fortran/62242
+	PR fortran/52332
+	* trans-array.c
+	(store_backend_decl): Create new gfc_charlen instance if requested
+	(get_array_ctor_all_strlen): Call store_backend_decl requesting
+	new gfc_charlen
+	(trans_array_constructor): Call store_backend_decl requesting
+	new gfc_charlen if get_array_ctor_strlen was called
+	(gfc_add_loop_ss_code): Don't try to convert non-constant length
+
 2015-09-28  Nathan Sidwell  <nathan@codesourcery.com>
 
 	* f95-lang.c (DEF_FUNCTION_TYPE_VAR_6): New.
Index: gcc/testsuite/ChangeLog
===================================================================
--- gcc/testsuite/ChangeLog	(revision 228220)
+++ gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,10 @@
+2015-09-28  Louis Krupp     <louis.krupp@zoho.com>
+
+	PR fortran/62242 fortran/52332
+	* gfortran.dg/string_array_constructor_1.f90: New.
+	* gfortran.dg/string_array_constructor_2.f90: New.
+	* gfortran.dg/string_array_constructor_3.f90: New.
+
 2015-09-28  Aditya Kumar  <aditya.k7@samsung.com>
 	    Sebastian Pop  <s.pop@samsung.com>
 
Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(revision 228220)
+++ gcc/fortran/trans-array.c	(working copy)
@@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p
 }
 
 
+/* The array constructor code can create a string length with an operand
+   in the form of a temporary variable.  This variable will retain its
+   context (current_function_decl).  If we store this length tree in a
+   gfc_charlen structure which is shared by a variable in another
+   context, the resulting gfc_charlen structure with a variable in a
+   different context, we could trip the assertion in expand_expr_real_1
+   when it sees that a variable has been created in one context and
+   referenced in another:
+
+      if (exp)
+	 context = decl_function_context (exp);
+      gcc_assert (!exp
+		  || SCOPE_FILE_SCOPE_P (context)
+		  || context == current_function_decl
+		  || TREE_STATIC (exp)
+		  || DECL_EXTERNAL (exp)
+		  // ??? C++ creates functions that are not TREE_STATIC.
+		  || TREE_CODE (exp) == FUNCTION_DECL);
+
+   If this might be the case, we create a new gfc_charlen structure and
+   link it into the current namespace.  */
+
+static void
+store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl)
+{
+  if (force_new_cl)
+    {
+      gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp);
+      *clp = new_cl;
+    }
+  (*clp)->backend_decl = len;
+}
+
 /* A catch-all to obtain the string length for anything that is not
    a substring of non-constant length, a constant, array or variable.  */
 
@@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc
       gfc_add_block_to_block (block, &se.pre);
       gfc_add_block_to_block (block, &se.post);
 
-      e->ts.u.cl->backend_decl = *len;
+      store_backend_decl (&e->ts.u.cl, *len, true);
     }
 }
 
@@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher
   if (expr->ts.type == BT_CHARACTER)
     {
       bool const_string;
+      bool force_new_cl = false;
 
       /* get_array_ctor_strlen walks the elements of the constructor, if a
 	 typespec was given, we already know the string length and want the one
@@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher
 	  gfc_add_block_to_block (&outer_loop->post, &length_se.post);
 	}
       else
-	const_string = get_array_ctor_strlen (&outer_loop->pre, c,
-					      &ss_info->string_length);
+	{
+	  const_string = get_array_ctor_strlen (&outer_loop->pre, c,
+						&ss_info->string_length);
+	  force_new_cl = true;
+	}
 
       /* Complex character array constructors should have been taken care of
 	 and not end up here.  */
       gcc_assert (ss_info->string_length);
 
-      expr->ts.u.cl->backend_decl = ss_info->string_length;
+      store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl);
 
       type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
       if (const_string)
@@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss
 	  if (expr->ts.type == BT_CHARACTER
 	      && ss_info->string_length == NULL
 	      && expr->ts.u.cl
-	      && expr->ts.u.cl->length)
+	      && expr->ts.u.cl->length
+	      && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
 	    {
 	      gfc_init_se (&se, NULL);
 	      gfc_conv_expr_type (&se, expr->ts.u.cl->length,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Re: Ping: New suggested patch for pr 62242 & pr 52332
  2015-09-29  7:36   ` Louis Krupp
@ 2015-10-01  9:10     ` Paul Richard Thomas
  2015-10-01  9:33       ` Louis Krupp
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Richard Thomas @ 2015-10-01  9:10 UTC (permalink / raw)
  To: Louis Krupp; +Cc: fortran, gcc-patches

Dear Louis,

I have just a minor nit to pick; otherwise your patch is OK for trunk.

I do not think that quoting the code in the first comment is
necessary. If anybody is interested, they can walz off to
expand_expr_real_1 themselves. The textual part of your comment is
perfectly clear.

Many thanks for the patch.

Paul

PS Do you now have all the paperwork to commit the patch yourself?

On 29 September 2015 at 09:36, Louis Krupp <louis.krupp@zoho.com> wrote:
> Paul,
>
> The patch is attached.  I last compiled and tested the code at 21:15 UTC on 28 September.
>
> Would you like me to resend the test cases?
>
> Louis
>
> ---- On Mon, 28 Sep 2015 00:55:10 -0700 Paul Richard Thomas  wrote ----
>>Hi Louis,
>>
>>Could you please send the patch as an attachment - in your message,
>>much of the lhs whitespace information has been lost. Fundamentally,
>>the patch looks OK. Since they pertain to the same PRs, I would
>>consider combining the first and third testcases, either by throwing
>>the procedures into one module of by renaming the modules; eg. gfbug1
>>and gfbug2.
>>
>>Cheers
>>
>>Paul
>>
>>On 25 September 2015 at 18:49, Louis Krupp <louis.krupp@zoho.com> wrote:
>>> Index: gcc/fortran/ChangeLog
>>> ===================================================================
>>> --- gcc/fortran/ChangeLog (revision 227895)
>>> +++ gcc/fortran/ChangeLog (working copy)
>>> @@ -1,3 +1,15 @@
>>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com>
>>> +
>>> + PR fortran/62242
>>> + PR fortran/52332
>>> + * trans-array.c
>>> + (store_backend_decl): Create new gfc_charlen instance if requested
>>> + (get_array_ctor_all_strlen): Call store_backend_decl requesting
>>> + new gfc_charlen
>>> + (trans_array_constructor): Call store_backend_decl requesting
>>> + new gfc_charlen if get_array_ctor_strlen was called
>>> + (gfc_add_loop_ss_code): Don't try to convert non-constant length
>>> +
>>> 2015-09-17 Paul Thomas <pault@gcc.gnu.org>
>>>
>>> PR fortran/52846
>>> Index: gcc/testsuite/ChangeLog
>>> ===================================================================
>>> --- gcc/testsuite/ChangeLog (revision 227895)
>>> +++ gcc/testsuite/ChangeLog (working copy)
>>> @@ -1,3 +1,10 @@
>>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com>
>>> +
>>> + PR fortran/62242 fortran/52332
>>> + * gfortran.dg/string_array_constructor_1.f90: New.
>>> + * gfortran.dg/string_array_constructor_2.f90: New.
>>> + * gfortran.dg/string_array_constructor_3.f90: New.
>>> +
>>> 2015-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de>
>>>
>>> PR sanitizer/64078
>>> Index: gcc/fortran/trans-array.c
>>> ===================================================================
>>> --- gcc/fortran/trans-array.c (revision 227895)
>>> +++ gcc/fortran/trans-array.c (working copy)
>>> @@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p
>>> }
>>>
>>>
>>> +/* The array constructor code can create a string length with an operand
>>> + in the form of a temporary variable. This variable will retain its
>>> + context (current_function_decl). If we store this length tree in a
>>> + gfc_charlen structure which is shared by a variable in another
>>> + context, the resulting gfc_charlen structure with a variable in a
>>> + different context, we could trip the assertion in expand_expr_real_1
>>> + when it sees that a variable has been created in one context and
>>> + referenced in another:
>>> +
>>> + if (exp)
>>> + context = decl_function_context (exp);
>>> + gcc_assert (!exp
>>> + || SCOPE_FILE_SCOPE_P (context)
>>> + || context == current_function_decl
>>> + || TREE_STATIC (exp)
>>> + || DECL_EXTERNAL (exp)
>>> + // ??? C++ creates functions that are not TREE_STATIC.
>>> + || TREE_CODE (exp) == FUNCTION_DECL);
>>> +
>>> + If this might be the case, we create a new gfc_charlen structure and
>>> + link it into the current namespace. */
>>> +
>>> +static void
>>> +store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl)
>>> +{
>>> + if (force_new_cl)
>>> + {
>>> + gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp);
>>> + *clp = new_cl;
>>> + }
>>> + (*clp)->backend_decl = len;
>>> +}
>>> +
>>> /* A catch-all to obtain the string length for anything that is not
>>> a substring of non-constant length, a constant, array or variable. */
>>>
>>> @@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc
>>> gfc_add_block_to_block (block, &se.pre);
>>> gfc_add_block_to_block (block, &se.post);
>>>
>>> - e->ts.u.cl->backend_decl = *len;
>>> + store_backend_decl (&e->ts.u.cl, *len, true);
>>> }
>>> }
>>>
>>> @@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher
>>> if (expr->ts.type == BT_CHARACTER)
>>> {
>>> bool const_string;
>>> + bool force_new_cl = false;
>>>
>>> /* get_array_ctor_strlen walks the elements of the constructor, if a
>>> typespec was given, we already know the string length and want the one
>>> @@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher
>>> gfc_add_block_to_block (&outer_loop->post, &length_se.post);
>>> }
>>> else
>>> - const_string = get_array_ctor_strlen (&outer_loop->pre, c,
>>> - &ss_info->string_length);
>>> + {
>>> + const_string = get_array_ctor_strlen (&outer_loop->pre, c,
>>> + &ss_info->string_length);
>>> + force_new_cl = true;
>>> + }
>>>
>>> /* Complex character array constructors should have been taken care of
>>> and not end up here. */
>>> gcc_assert (ss_info->string_length);
>>>
>>> - expr->ts.u.cl->backend_decl = ss_info->string_length;
>>> + store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl);
>>>
>>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length);
>>> if (const_string)
>>> @@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss
>>> if (expr->ts.type == BT_CHARACTER
>>> && ss_info->string_length == NULL
>>> && expr->ts.u.cl
>>> - && expr->ts.u.cl->length)
>>> + && expr->ts.u.cl->length
>>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT)
>>> {
>>> gfc_init_se (&se, NULL);
>>> gfc_conv_expr_type (&se, expr->ts.u.cl->length,
>>
>>
>>
>>--
>>Outside of a dog, a book is a man's best friend. Inside of a dog it's
>>too dark to read.
>>
>>Groucho Marx
>>



-- 
Outside of a dog, a book is a man's best friend. Inside of a dog it's
too dark to read.

Groucho Marx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Re: Re: Ping: New suggested patch for pr 62242 & pr 52332
  2015-10-01  9:10     ` Paul Richard Thomas
@ 2015-10-01  9:33       ` Louis Krupp
  0 siblings, 0 replies; 5+ messages in thread
From: Louis Krupp @ 2015-10-01  9:33 UTC (permalink / raw)
  To: Paul Richard Thomas; +Cc: fortran, gcc-patches

Paul,

I'll delete the code in the comment.

I'm doing one more update and build just to make sure everything still works as intended.

I'm not sure what paperwork I need.  Shall I try to do "svn commit" and see what happens?

Louis

---- On Thu, 01 Oct 2015 02:10:34 -0700 Paul Richard Thomas  wrote ---- 
>Dear Louis, 
> 
>I have just a minor nit to pick; otherwise your patch is OK for trunk. 
> 
>I do not think that quoting the code in the first comment is 
>necessary. If anybody is interested, they can walz off to 
>expand_expr_real_1 themselves. The textual part of your comment is 
>perfectly clear. 
> 
>Many thanks for the patch. 
> 
>Paul 
> 
>PS Do you now have all the paperwork to commit the patch yourself? 
> 
>On 29 September 2015 at 09:36, Louis Krupp <louis.krupp@zoho.com> wrote: 
>> Paul, 
>> 
>> The patch is attached. I last compiled and tested the code at 21:15 UTC on 28 September. 
>> 
>> Would you like me to resend the test cases? 
>> 
>> Louis 
>> 
>> ---- On Mon, 28 Sep 2015 00:55:10 -0700 Paul Richard Thomas wrote ---- 
>>>Hi Louis, 
>>> 
>>>Could you please send the patch as an attachment - in your message, 
>>>much of the lhs whitespace information has been lost. Fundamentally, 
>>>the patch looks OK. Since they pertain to the same PRs, I would 
>>>consider combining the first and third testcases, either by throwing 
>>>the procedures into one module of by renaming the modules; eg. gfbug1 
>>>and gfbug2. 
>>> 
>>>Cheers 
>>> 
>>>Paul 
>>> 
>>>On 25 September 2015 at 18:49, Louis Krupp <louis.krupp@zoho.com> wrote: 
>>>> Index: gcc/fortran/ChangeLog 
>>>> =================================================================== 
>>>> --- gcc/fortran/ChangeLog (revision 227895) 
>>>> +++ gcc/fortran/ChangeLog (working copy) 
>>>> @@ -1,3 +1,15 @@ 
>>>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com> 
>>>> + 
>>>> + PR fortran/62242 
>>>> + PR fortran/52332 
>>>> + * trans-array.c 
>>>> + (store_backend_decl): Create new gfc_charlen instance if requested 
>>>> + (get_array_ctor_all_strlen): Call store_backend_decl requesting 
>>>> + new gfc_charlen 
>>>> + (trans_array_constructor): Call store_backend_decl requesting 
>>>> + new gfc_charlen if get_array_ctor_strlen was called 
>>>> + (gfc_add_loop_ss_code): Don't try to convert non-constant length 
>>>> + 
>>>> 2015-09-17 Paul Thomas <pault@gcc.gnu.org> 
>>>> 
>>>> PR fortran/52846 
>>>> Index: gcc/testsuite/ChangeLog 
>>>> =================================================================== 
>>>> --- gcc/testsuite/ChangeLog (revision 227895) 
>>>> +++ gcc/testsuite/ChangeLog (working copy) 
>>>> @@ -1,3 +1,10 @@ 
>>>> +2015-09-18 Louis Krupp <louis.krupp@zoho.com> 
>>>> + 
>>>> + PR fortran/62242 fortran/52332 
>>>> + * gfortran.dg/string_array_constructor_1.f90: New. 
>>>> + * gfortran.dg/string_array_constructor_2.f90: New. 
>>>> + * gfortran.dg/string_array_constructor_3.f90: New. 
>>>> + 
>>>> 2015-09-17 Bernd Edlinger <bernd.edlinger@hotmail.de> 
>>>> 
>>>> PR sanitizer/64078 
>>>> Index: gcc/fortran/trans-array.c 
>>>> =================================================================== 
>>>> --- gcc/fortran/trans-array.c (revision 227895) 
>>>> +++ gcc/fortran/trans-array.c (working copy) 
>>>> @@ -1799,6 +1799,39 @@ gfc_trans_array_constructor_value (stmtblock_t * p 
>>>> } 
>>>> 
>>>> 
>>>> +/* The array constructor code can create a string length with an operand 
>>>> + in the form of a temporary variable. This variable will retain its 
>>>> + context (current_function_decl). If we store this length tree in a 
>>>> + gfc_charlen structure which is shared by a variable in another 
>>>> + context, the resulting gfc_charlen structure with a variable in a 
>>>> + different context, we could trip the assertion in expand_expr_real_1 
>>>> + when it sees that a variable has been created in one context and 
>>>> + referenced in another: 
>>>> + 
>>>> + if (exp) 
>>>> + context = decl_function_context (exp); 
>>>> + gcc_assert (!exp 
>>>> + || SCOPE_FILE_SCOPE_P (context) 
>>>> + || context == current_function_decl 
>>>> + || TREE_STATIC (exp) 
>>>> + || DECL_EXTERNAL (exp) 
>>>> + // ??? C++ creates functions that are not TREE_STATIC. 
>>>> + || TREE_CODE (exp) == FUNCTION_DECL); 
>>>> + 
>>>> + If this might be the case, we create a new gfc_charlen structure and 
>>>> + link it into the current namespace. */ 
>>>> + 
>>>> +static void 
>>>> +store_backend_decl (gfc_charlen **clp, tree len, bool force_new_cl) 
>>>> +{ 
>>>> + if (force_new_cl) 
>>>> + { 
>>>> + gfc_charlen *new_cl = gfc_new_charlen (gfc_current_ns, *clp); 
>>>> + *clp = new_cl; 
>>>> + } 
>>>> + (*clp)->backend_decl = len; 
>>>> +} 
>>>> + 
>>>> /* A catch-all to obtain the string length for anything that is not 
>>>> a substring of non-constant length, a constant, array or variable. */ 
>>>> 
>>>> @@ -1836,7 +1869,7 @@ get_array_ctor_all_strlen (stmtblock_t *block, gfc 
>>>> gfc_add_block_to_block (block, &se.pre); 
>>>> gfc_add_block_to_block (block, &se.post); 
>>>> 
>>>> - e->ts.u.cl->backend_decl = *len; 
>>>> + store_backend_decl (&e->ts.u.cl, *len, true); 
>>>> } 
>>>> } 
>>>> 
>>>> @@ -2226,6 +2259,7 @@ trans_array_constructor (gfc_ss * ss, locus * wher 
>>>> if (expr->ts.type == BT_CHARACTER) 
>>>> { 
>>>> bool const_string; 
>>>> + bool force_new_cl = false; 
>>>> 
>>>> /* get_array_ctor_strlen walks the elements of the constructor, if a 
>>>> typespec was given, we already know the string length and want the one 
>>>> @@ -2244,14 +2278,17 @@ trans_array_constructor (gfc_ss * ss, locus * wher 
>>>> gfc_add_block_to_block (&outer_loop->post, &length_se.post); 
>>>> } 
>>>> else 
>>>> - const_string = get_array_ctor_strlen (&outer_loop->pre, c, 
>>>> - &ss_info->string_length); 
>>>> + { 
>>>> + const_string = get_array_ctor_strlen (&outer_loop->pre, c, 
>>>> + &ss_info->string_length); 
>>>> + force_new_cl = true; 
>>>> + } 
>>>> 
>>>> /* Complex character array constructors should have been taken care of 
>>>> and not end up here. */ 
>>>> gcc_assert (ss_info->string_length); 
>>>> 
>>>> - expr->ts.u.cl->backend_decl = ss_info->string_length; 
>>>> + store_backend_decl (&expr->ts.u.cl, ss_info->string_length, force_new_cl); 
>>>> 
>>>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length); 
>>>> if (const_string) 
>>>> @@ -2589,7 +2626,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loop, gfc_ss 
>>>> if (expr->ts.type == BT_CHARACTER 
>>>> && ss_info->string_length == NULL 
>>>> && expr->ts.u.cl 
>>>> - && expr->ts.u.cl->length) 
>>>> + && expr->ts.u.cl->length 
>>>> + && expr->ts.u.cl->length->expr_type == EXPR_CONSTANT) 
>>>> { 
>>>> gfc_init_se (&se, NULL); 
>>>> gfc_conv_expr_type (&se, expr->ts.u.cl->length, 
>>> 
>>> 
>>> 
>>>-- 
>>>Outside of a dog, a book is a man's best friend. Inside of a dog it's 
>>>too dark to read. 
>>> 
>>>Groucho Marx 
>>> 
> 
> 
> 
>-- 
>Outside of a dog, a book is a man's best friend. Inside of a dog it's 
>too dark to read. 
> 
>Groucho Marx 
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-10-01  9:33 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-26  9:18 Ping: New suggested patch for pr 62242 & pr 52332 Louis Krupp
2015-09-28  7:55 ` Paul Richard Thomas
2015-09-29  7:36   ` Louis Krupp
2015-10-01  9:10     ` Paul Richard Thomas
2015-10-01  9:33       ` Louis Krupp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).