Variable is not referenced in report (SVN ticket : #863) #94

SBelondrade · 2023-03-14T22:02:16Z

Hi Simon, I'm using my second account for this pull request.

Reference svn ticket : #863.

I replaced printf by cb_warning_x but I think I have some problems of understanding.

If I understand, I have to remove two checks and this condition in codegen.c :

if((f->report_flag & COB_REPORT_LINE)
	&& f->children
	&& (f->children->report_flag & COB_REPORT_LINE)) {
		printf("Warning: Ignoring nested LINE %s %d\n",
			(f->report_flag & COB_REPORT_LINE_PLUS)?"PLUS":"",
			f->report_line);
		f->report_line = 0;
		f->report_flag &= ~COB_REPORT_LINE_PLUS;
		f->report_flag &= ~COB_REPORT_LINE;
	}

I added a new function and I called it finalize_report :

static  void
check_report (struct  cb_report *r, cb_tree  ctl)
{
	if (!ctl) {
		return ;
	}
	
	cb_tree  nx = CB_CHAIN(ctl);
	
	if (nx) {
		check_report(r, nx);
	}
	int  bfound = 0;
	cb_tree  x = CB_VALUE (ctl);
	struct  cb_field *s = cb_code_field(x);
	if((s->report_flag & COB_REPORT_LINE)
	  && s->children
	  && (s->children->report_flag & COB_REPORT_LINE)) {
		cb_warning_x (COBC_WARN_FILLER,
		CB_TREE(s), _("Warning: Ignoring nested LINE %s  %d"),
		  (s->report_flag & COB_REPORT_LINE_PLUS)?"PLUS":"",
			s->report_line);
			s->report_line = 0;
			s->report_flag &= ~COB_REPORT_LINE_PLUS;
			s->report_flag &= ~COB_REPORT_LINE;
	}
	for(int  i = r->num_lines-1; i >= 0; i--) {
		if(r->line_ids[i]->report_control) {
			struct  cb_field *c = cb_code_field (r->line_ids[i]->report_control);
			if(c == s) {
				bfound = 1;
				break;
			}
		}
	}
	if (!bfound) {
		cb_warning_x (COBC_WARN_FILLER,
		CB_TREE(ctl), _("Control field %s is not referenced in report"),
			s->name);
		ctl = NULL;
	}
}

But I don't pass some unit test. Please tell me if I haven´t understood what I had to do ?

lefessan · 2023-03-19T19:20:10Z

cobc/codegen.c

+	if (!bfound) {
+		cb_warning_x (COBC_WARN_FILLER,
+			CB_TREE(ctl), _("Control field %s is not referenced in report"), s->name);
+		ctl = NULL;


ctl = NULL; is useless as ctl is a local variable.

lefessan · 2023-03-19T19:31:03Z

cobc/codegen.c

+			CB_TREE(ctl), _("Control field %s is not referenced in report"), s->name);
+		ctl = NULL;
+		p->controls = NULL;
+		return ;


This won't work if there is a list of more than one controls, and the first one is not used. If you only issue a warning and not an error, your code has to handle correctly such a case.

I would advise to clean up the controls at the beginning of the output_report_definition function, so that p->controls only contains controls that are used within the report.

Just to clarify: output_control_report is called on nx recursively just before your test, for other controls, so if you set p->controls = NULL, you discard all of them. The list of controls is used several times, so doing the test every time would be too complex, the only solution I see is to cleanup the list once at the beginning.

GitMensch

See my review comments for reasons that may be related to the test suite. Sorry for not paying that earlier (while I wrote those notes directly after you PR, I somehow missed to click on "submit review").

Furthermore please rebase.

GitMensch · 2023-03-16T14:38:19Z

cobc/codegen.c

@@ -9727,7 +9746,8 @@ output_report_control (struct cb_report *p, int id, cb_tree ctl, cb_tree nx)
 		}
 	}
 	if(!bfound) {
-		printf("Control field %s is not referenced in report\n",s->name);
+		cb_warning_x (COBC_WARN_FILLER,
+			CB_TREE(ctl), _("Control field %s is not referenced in report"), s->name);


Do we need both this and the part above? Seem duplicated.

Please start (nearly every) diagnostic with a lower-case letter; the only reason that this did not showed itself before is that it wasn't marked as msgid (the _() part that leads to translatable strings).

GitMensch · 2023-03-16T14:40:43Z

cobc/codegen.c

@@ -10023,7 +10043,8 @@ output_report_define_lines (int top, struct cb_field *f, struct cb_report *r)
 	if ((f->report_flag & COB_REPORT_LINE)
 	&& f->children
 	&& (f->children->report_flag & COB_REPORT_LINE)) {
-		printf("Warning: Ignoring nested LINE %s %d\n",
+		cb_warning_x (COBC_WARN_FILLER,
+			CB_TREE(f), _("Warning: Ignoring nested LINE %s %d"),


The "warning" prefix already is in by cb_warning, pleas drop that and start with lowercase letter.

GitMensch · 2023-03-16T14:41:48Z

tests/testsuite.src/run_reportwriter.at

+                   09   ERROR-2     PIC 9(3).
+])
+
+AT_CHECK([$COMPILE prog.cob], [0], [], [prog.cob:18: warning: Control field ERROR-1 is not referenced in report


just minor style: please add a line break after [],

GitMensch · 2023-03-16T14:44:03Z

tests/testsuite.src/run_reportwriter.at

@@ -9626,3 +9626,49 @@ BEFORE FINAL - SHOULD DISPLAY

 AT_CLEANUP

+AT_SETUP([Check if the variable is referenced in the report])


This is just a syntax check, please move to syn_reportwriter, using $COMPILE_ONLY.

Please also add a test for the "ignoring nested line".

codecov-commenter · 2023-07-21T15:47:11Z

Codecov Report

Merging #94 (bb2edcf) into gcos4gnucobol-3.x (6b44051) will increase coverage by 0.01%.
The diff coverage is 88.23%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x      #94      +/-   ##
=====================================================
+ Coverage              65.39%   65.40%   +0.01%     
=====================================================
  Files                     32       32              
  Lines                  58797    58808      +11     
  Branches               15492    15496       +4     
=====================================================
+ Hits                   38449    38463      +14     
+ Misses                 14362    14361       -1     
+ Partials                5986     5984       -2

Impacted Files	Coverage Δ
cobc/codegen.c	`75.39% <88.23%> (+0.06%)`	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

GitMensch

Rechecked: this issue still happens both in 3.2 release (and therefore current 3.x branch) and trunk. it would be nice if this PR can be finished so that we can include it upstream.

GitMensch · 2023-08-25T07:05:54Z

cobc/codegen.c

+			}
+		}
+	}
+	if (!bfound) {


This check, the warning and likely also the setting to null (or dropping the single unused control, as @lefessan pointed out) must be moved to typeck.c.

Ok. I moved this

GitMensch · 2023-08-25T07:17:48Z

tests/testsuite.src/syn_reportwriter.at

+AT_CHECK([$COMPILE prog.cob], [0], [],
+[prog.cob: warning: control field ERROR-1 is not referenced in report
+prog.cob: warning: control field ERROR-2 is not referenced in report
+])


This ideally should be changed to

AT_CHECK([$COMPILE_ONLY prog.cob], [0], [], [prog.cob: warning: control field ERROR-1 is not referenced in report prog.cob: warning: control field ERROR-2 is not referenced in report ]) # additional check if we generate code that compiles, see bug #863 AT_CHECK([$COMPILE -w prog.cob], [0], [], [])

GitMensch · 2023-08-25T07:20:04Z

cobc/codegen.c

@@ -10229,7 +10246,8 @@ output_report_define_lines (int top, struct cb_field *f, struct cb_report *r)
 	if ((f->report_flag & COB_REPORT_LINE)


(originating lines 10229-10235) This should be moved to typeck.c, at least the warning.

dropped trailing whiutespace - mostly to get the CI running...

sbelondr added 3 commits March 14, 2023 14:56

replace printf by cb_warning_x

78a6057

add unit test

7b18452

rm Makefile

610819d

lefessan requested changes Mar 19, 2023

View reviewed changes

GitMensch requested changes Jun 13, 2023

View reviewed changes

sbelondr and others added 2 commits July 21, 2023 17:04

Replace cb_warning_x by cb_warning and mode unit test

7402d7b

Merge branch 'gcos4gnucobol-3.x' into gcos4gnucobol-3.x

bb2edcf

GitMensch requested changes Aug 25, 2023

View reviewed changes

sbelondr and others added 2 commits September 7, 2023 16:18

add check in syn_reportwriter.at and move warning in the typeck.c file

86c8cac

Update ChangeLog

15ecc19

dropped trailing whiutespace - mostly to get the CI running...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Variable is not referenced in report (SVN ticket : #863) #94

Variable is not referenced in report (SVN ticket : #863) #94

SBelondrade commented Mar 14, 2023

lefessan Mar 19, 2023

lefessan Mar 19, 2023

lefessan Mar 19, 2023

GitMensch left a comment

GitMensch Mar 16, 2023

GitMensch Mar 16, 2023

GitMensch Mar 16, 2023

GitMensch Mar 16, 2023

GitMensch Mar 16, 2023

codecov-commenter commented Jul 21, 2023

GitMensch left a comment

GitMensch Aug 25, 2023

sbelondr Sep 7, 2023

GitMensch Aug 25, 2023

GitMensch Aug 25, 2023

		@@ -9626,3 +9626,49 @@ BEFORE FINAL - SHOULD DISPLAY

		AT_CLEANUP

		AT_SETUP([Check if the variable is referenced in the report])

		@@ -10229,7 +10246,8 @@ output_report_define_lines (int top, struct cb_field f, struct cb_report r)
		if ((f->report_flag & COB_REPORT_LINE)

Variable is not referenced in report (SVN ticket : #863) #94

Are you sure you want to change the base?

Variable is not referenced in report (SVN ticket : #863) #94

Conversation

SBelondrade commented Mar 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GitMensch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 21, 2023

Codecov Report

GitMensch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment