Re: Re: Possible patch for pr62242 -- follow-up

[Louis Krupp <louis.krupp@zoho.com>]

Paul & Dominique,

I've been taking another look at this, too, and I'd like to have another go at understanding what's happening.

On mature reflection, my "if (0)" near line 1836 seems like a bad idea.

The assertion at 2251 is copied from expr.c, and it's in a comment that attempts to explain why the code is creating a new gfc_charlen structure.  Unless I can figure out a simpler way to fix this, I'd like to leave the comment as it is, because I think someone might have to revisit this code sooner or later.

Louis

---- On Wed, 16 Sep 2015 01:58:59 -0700 Paul Richard Thomas  wrote ---- 
>Dear Louis and Dominique, 
> 
>I will take a look at your patch tonight and will attempt to answer 
>Dominique's two questions in the process. 
> 
>A comment: Does the assertion in the hunk at 2251 not cast such a wide 
>net as to make it redundant? 
> 
>Cheers 
> 
>Paul 
> 
>On 15 September 2015 at 15:35, Dominique d'Humières <dominiq@lps.ens.fr> wrote: 
>> Louis, 
>> 
>> A few comments: 
>> 
>> (1) your patch fixes pr62242 and, as said in a previous mail, pr62246, pr60110, and some variants of pr52332. I have updated pr52332 and marked pr62246 and pr60110 as duplicate. Could you please add 
>> 
>> PR fortran/52332 
>> 
>> to the ChangeLog entry? 
>> 
>> (2) You need a ChangeLog entry for the tests in gcc/testsuite. 
>> 
>> (3) The usual way to attract attention is a PING in the subject. 
>> 
>> (4) AFAICT your patch boils down to 
>> 
>> --- ../_clean/gcc/fortran/trans-array.c 2015-08-29 10:47:51.000000000 +0200 
>> +++ gcc/fortran/trans-array.c 2015-09-15 15:21:16.000000000 +0200 
>> @@ -1835,8 +1835,6 @@ get_array_ctor_all_strlen (stmtblock_t * 
>> 
>> gfc_add_block_to_block (block, &se.pre); 
>> gfc_add_block_to_block (block, &se.post); 
>> - 
>> - e->ts.u.cl->backend_decl = *len; 
>> } 
>> } 
>> 
>> @@ -2226,6 +2224,7 @@ trans_array_constructor (gfc_ss * ss, lo 
>> if (expr->ts.type == BT_CHARACTER) 
>> { 
>> bool const_string; 
>> + gfc_charlen *new_cl; 
>> 
>> /* 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 
>> @@ -2251,7 +2250,9 @@ trans_array_constructor (gfc_ss * ss, lo 
>> and not end up here. */ 
>> gcc_assert (ss_info->string_length); 
>> 
>> - expr->ts.u.cl->backend_decl = ss_info->string_length; 
>> + new_cl = gfc_new_charlen (gfc_current_ns, expr->ts.u.cl); 
>> + new_cl->backend_decl = ss_info->string_length; 
>> + expr->ts.u.cl = new_cl; 
>> 
>> type = gfc_get_character_type_len (expr->ts.kind, ss_info->string_length); 
>> if (const_string) 
>> @@ -2589,7 +2590,8 @@ gfc_add_loop_ss_code (gfc_loopinfo * loo 
>> 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, 
>> 
>> with two questions: 
>> 
>> (a) Is the line 
>> 
>> e->ts.u.cl->backend_decl = *len; 
>> 
>> needed? It does not for my own tests (although this may be masked by errors or other ICEs) 
>> 
>> (b) A long question for the hunk at 2251 I don’t understand (thus cannot answer). 
>> 
>> Thanks for the patch, 
>> 
>> Dominique 
>> 
> 
> 
> 
>-- 
>Outside of a dog, a book is a man's best friend. Inside of a dog it's 
>too dark to read. 
> 
>Groucho Marx 
>