diff mbox

[fortran,RFC] Add warning for missing location information

Message ID 20161206144240.03c8014e@vepi2
State New
Headers show

Commit Message

Andre Vehreschild Dec. 6, 2016, 1:42 p.m. UTC 
Hi Thomas, hi all,

my part of the patch was okayed by Thomas in the PR at comment #12:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78226#c12

Committed as r243300.

Thomas, your part of the patch looks ok to me, too. I haven't tracked the
discussion on whether it is ok to add your part of the patch. Therefore I don't
have committed your patch. My opinion is, that it does do no harm. So it should
be applied.

Regards and thanks,
	Andre

On Mon, 5 Dec 2016 19:02:32 +0100
Andre Vehreschild <vehre@gmx.de> wrote:

> Hi Thomas,

> 

> attached patch fixes the remaining open issues on missing locus information

> (in class-finalizers, to be precise), at least on my installation for

> 

> make check-fortran

> 

> For libgomp there are still a lot of regressions reported. Given some time I

> look into it.

> 

> - Andre

> 

> On Sun, 4 Dec 2016 15:32:21 +0100

> Thomas Koenig <tkoenig@netcologne.de> wrote:

> 

> > Am 04.12.2016 um 12:40 schrieb Thomas Koenig:

> >   

> > > So, not OK for trunk at the moment.  OK once failures have

> > > been fixed?    

> > 

> > 

> > This version of the patch actually works (it includes the

> > change to gfortran.h).

> > 

> > Regards

> > 

> > 	Thomas  

> 

> 



-- 
Andre Vehreschild * Email: vehre ad gmx dot de

Comments

Thomas Koenig Dec. 8, 2016, 7:26 p.m. UTC | #1 
Hi Andre,

> Thomas, your part of the patch looks ok to me, too. I haven't tracked the

> discussion on whether it is ok to add your part of the patch. Therefore I don't

> have committed your patch. My opinion is, that it does do no harm. So it should

> be applied.


Based on the feedback so far, I will this apply during the weekend
after another regression test unless somebody objects.

Regards

	Thomas
Mikael Morin Dec. 8, 2016, 8:01 p.m. UTC | #2 
Le 08/12/2016 à 20:26, Thomas Koenig a écrit :
> Hi Andre,

>

>> Thomas, your part of the patch looks ok to me, too. I haven't tracked the

>> discussion on whether it is ok to add your part of the patch.

>> Therefore I don't

>> have committed your patch. My opinion is, that it does do no harm. So

>> it should

>> be applied.

>

> Based on the feedback so far, I will this apply during the weekend

> after another regression test unless somebody objects.

>


Hello,

I have one comment.

This error;
+    gfc_warning_internal (0, "No location in statement statement");

is not very helpful, especially without location.
Could you at least provide the statement type? There is 
gfc_ascii_statement you can use for that.

Thanks
Mikael
Thomas Koenig Dec. 8, 2016, 9:50 p.m. UTC | #3 
Am 08.12.2016 um 21:01 schrieb Mikael Morin:
> Le 08/12/2016 à 20:26, Thomas Koenig a écrit :

>> Hi Andre,

>>

>>> Thomas, your part of the patch looks ok to me, too. I haven't tracked

>>> the

>>> discussion on whether it is ok to add your part of the patch.

>>> Therefore I don't

>>> have committed your patch. My opinion is, that it does do no harm. So

>>> it should

>>> be applied.

>>

>> Based on the feedback so far, I will this apply during the weekend

>> after another regression test unless somebody objects.

>>

>

> Hello,

>

> I have one comment.

>

> This error;

> +    gfc_warning_internal (0, "No location in statement statement");

>

> is not very helpful, especially without location.


> Could you at least provide the statement type? There is

> gfc_ascii_statement you can use for that.


gfc_ascii_statement works with gfc_statement only,
here, we are dealing with gfc_exec_op.

What I could do is to add a gfc_debug_code function to
dump-parse-tree and add a comment that, if this part of
the code is encountered, you should examine the expression
using that particular function call using a debugger.

Of course, I could also just dump the statement to stderr,
but somehow that does not quite conform to all the elegang
error handling infrastructure in gcc :-)

So, what do people think?

Regards

	Thomas
Mikael Morin Dec. 8, 2016, 10:04 p.m. UTC | #4 
Le 08/12/2016 à 22:50, Thomas Koenig a écrit :
> Am 08.12.2016 um 21:01 schrieb Mikael Morin:

>> Le 08/12/2016 à 20:26, Thomas Koenig a écrit :

>>> Hi Andre,

>>>

>>>> Thomas, your part of the patch looks ok to me, too. I haven't tracked

>>>> the

>>>> discussion on whether it is ok to add your part of the patch.

>>>> Therefore I don't

>>>> have committed your patch. My opinion is, that it does do no harm. So

>>>> it should

>>>> be applied.

>>>

>>> Based on the feedback so far, I will this apply during the weekend

>>> after another regression test unless somebody objects.

>>>

>>

>> Hello,

>>

>> I have one comment.

>>

>> This error;

>> +    gfc_warning_internal (0, "No location in statement statement");

>>

>> is not very helpful, especially without location.

>

>> Could you at least provide the statement type? There is

>> gfc_ascii_statement you can use for that.

>

> gfc_ascii_statement works with gfc_statement only,

> here, we are dealing with gfc_exec_op.

>

Of course. :-(

> What I could do is to add a gfc_debug_code function to

> dump-parse-tree and add a comment that, if this part of

> the code is encountered, you should examine the expression

> using that particular function call using a debugger.

>

> Of course, I could also just dump the statement to stderr,

> but somehow that does not quite conform to all the elegang

> error handling infrastructure in gcc :-)

>

Yes, please don’t do that.

> So, what do people think?

>

As the code is disabled for releases anyway, I don’t mind the patch 
getting in as is. So OK from my side.
diff mbox

Patch

Index: gcc/fortran/ChangeLog
===================================================================
--- gcc/fortran/ChangeLog	(Revision 243297)
+++ gcc/fortran/ChangeLog	(Arbeitskopie)
@@ -1,3 +1,12 @@ 
+2016-12-06  Andre Vehreschild  <vehre@gcc.gnu.org>
+
+	PR fortran/78226
+	* class.c (finalize_component): Add missing locus information.
+	(finalization_scalarizer): Likewise.
+	(finalization_get_offset): Likewise.
+	(finalizer_insert_packed_call): Likewise.
+	(generate_finalization_wrapper): Likewise.
+
 2016-12-05  Nathan Sidwell  <nathan@acm.org>
 
 	* error.c (gfc_warning_check): Call diagnostic_check_max_errors.
Index: gcc/fortran/class.c
===================================================================
--- gcc/fortran/class.c	(Revision 243297)
+++ gcc/fortran/class.c	(Arbeitskopie)
@@ -965,6 +965,7 @@ 
       cond->block = gfc_get_code (EXEC_IF);
       cond->block->expr1 = gfc_get_expr ();
       cond->block->expr1->expr_type = EXPR_FUNCTION;
+      cond->block->expr1->where = gfc_current_locus;
       gfc_get_sym_tree ("associated", sub_ns, &cond->block->expr1->symtree, false);
       cond->block->expr1->symtree->n.sym->attr.flavor = FL_PROCEDURE;
       cond->block->expr1->symtree->n.sym->attr.intrinsic = 1;
@@ -1077,6 +1078,7 @@ 
   gfc_commit_symbol (expr->symtree->n.sym);
   expr->ts.type = BT_INTEGER;
   expr->ts.kind = gfc_index_integer_kind;
+  expr->where = gfc_current_locus;
 
   /* TRANSFER.  */
   expr2 = gfc_build_intrinsic_call (sub_ns, GFC_ISYM_TRANSFER, "transfer",
@@ -1093,6 +1095,7 @@ 
   block->ext.actual->expr->value.op.op1 = expr2;
   block->ext.actual->expr->value.op.op2 = offset;
   block->ext.actual->expr->ts = expr->ts;
+  block->ext.actual->expr->where = gfc_current_locus;
 
   /* C_F_POINTER's 2nd arg: ptr -- and its absent shape=.  */
   block->ext.actual->next = gfc_get_actual_arglist ();
@@ -1149,6 +1152,7 @@ 
   expr->ref->u.ar.dimen = 1;
   expr->ref->u.ar.dimen_type[0] = DIMEN_ELEMENT;
   expr->ref->u.ar.start[0] = gfc_lval_expr_from_sym (idx2);
+  expr->where = sizes->declared_at;
 
   expr = gfc_build_intrinsic_call (sub_ns, GFC_ISYM_MOD, "mod",
 				   gfc_current_locus, 2,
@@ -1169,6 +1173,7 @@ 
   expr2->value.op.op2->ref->u.ar.dimen_type[0] = DIMEN_ELEMENT;
   expr2->value.op.op2->ref->u.ar.start[0] = gfc_get_expr ();
   expr2->value.op.op2->ref->u.ar.start[0]->expr_type = EXPR_OP;
+  expr2->value.op.op2->ref->u.ar.start[0]->where = gfc_current_locus;
   expr2->value.op.op2->ref->u.ar.start[0]->value.op.op = INTRINSIC_MINUS;
   expr2->value.op.op2->ref->u.ar.start[0]->value.op.op1
 	= gfc_lval_expr_from_sym (idx2);
@@ -1177,6 +1182,7 @@ 
   expr2->value.op.op2->ref->u.ar.start[0]->ts
 	= expr2->value.op.op2->ref->u.ar.start[0]->value.op.op1->ts;
   expr2->ts = idx->ts;
+  expr2->where = gfc_current_locus;
 
   /* ... * strides(idx2).  */
   expr = gfc_get_expr ();
@@ -1192,6 +1198,7 @@ 
   expr->value.op.op2->ref->u.ar.start[0] = gfc_lval_expr_from_sym (idx2);
   expr->value.op.op2->ref->u.ar.as = strides->as;
   expr->ts = idx->ts;
+  expr->where = gfc_current_locus;
 
   /* offset = offset + ...  */
   block->block->next = gfc_get_code (EXEC_ASSIGN);
@@ -1202,6 +1209,7 @@ 
   block->block->next->expr2->value.op.op1 = gfc_lval_expr_from_sym (offset);
   block->block->next->expr2->value.op.op2 = expr;
   block->block->next->expr2->ts = idx->ts;
+  block->block->next->expr2->where = gfc_current_locus;
 
   /* After the loop:  offset = offset * byte_stride.  */
   block->next = gfc_get_code (EXEC_ASSIGN);
@@ -1213,6 +1221,7 @@ 
   block->expr2->value.op.op1 = gfc_lval_expr_from_sym (offset);
   block->expr2->value.op.op2 = gfc_lval_expr_from_sym (byte_stride);
   block->expr2->ts = block->expr2->value.op.op1->ts;
+  block->expr2->where = gfc_current_locus;
   return block;
 }
 
@@ -1422,6 +1431,7 @@ 
   /* Offset calculation for the new array: idx * size of type (in bytes).  */
   offset2 = gfc_get_expr ();
   offset2->expr_type = EXPR_OP;
+  offset2->where = gfc_current_locus;
   offset2->value.op.op = INTRINSIC_TIMES;
   offset2->value.op.op1 = gfc_lval_expr_from_sym (idx);
   offset2->value.op.op2 = gfc_copy_expr (size_expr);
@@ -1826,6 +1836,7 @@ 
   block->expr2 = gfc_get_expr ();
   block->expr2->expr_type = EXPR_OP;
   block->expr2->value.op.op = INTRINSIC_TIMES;
+  block->expr2->where = gfc_current_locus;
 
   /* sizes(idx-1).  */
   block->expr2->value.op.op1 = gfc_lval_expr_from_sym (sizes);
@@ -1837,6 +1848,7 @@ 
   block->expr2->value.op.op1->ref->u.ar.dimen_type[0] = DIMEN_ELEMENT;
   block->expr2->value.op.op1->ref->u.ar.start[0] = gfc_get_expr ();
   block->expr2->value.op.op1->ref->u.ar.start[0]->expr_type = EXPR_OP;
+  block->expr2->value.op.op1->ref->u.ar.start[0]->where = gfc_current_locus;
   block->expr2->value.op.op1->ref->u.ar.start[0]->value.op.op = INTRINSIC_MINUS;
   block->expr2->value.op.op1->ref->u.ar.start[0]->value.op.op1
 	= gfc_lval_expr_from_sym (idx);
@@ -1890,6 +1902,7 @@ 
   block->expr1->value.op.op2->ref->u.ar.dimen_type[0] = DIMEN_ELEMENT;
   block->expr1->value.op.op2->ref->u.ar.start[0] = gfc_get_expr ();
   block->expr1->value.op.op2->ref->u.ar.start[0]->expr_type = EXPR_OP;
+  block->expr1->value.op.op2->ref->u.ar.start[0]->where = gfc_current_locus;
   block->expr1->value.op.op2->ref->u.ar.start[0]->value.op.op = INTRINSIC_MINUS;
   block->expr1->value.op.op2->ref->u.ar.start[0]->value.op.op1
 	= gfc_lval_expr_from_sym (idx);
@@ -1927,6 +1940,7 @@ 
   last_code->expr2->value.op.op2
 	= gfc_get_int_expr (gfc_index_integer_kind, NULL, 1);
   last_code->expr2->ts = last_code->expr2->value.op.op2->ts;
+  last_code->expr2->where = gfc_current_locus;
 
   last_code->expr2->value.op.op1 = gfc_lval_expr_from_sym (sizes);
   last_code->expr2->value.op.op1->ref = gfc_get_ref ();