Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: gcos4gnucobol-3.x
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
78a6057
replace printf by cb_warning_x
sbelondr Mar 14, 2023
7b18452
add unit test
sbelondr Mar 14, 2023
610819d
rm Makefile
sbelondr Mar 14, 2023
7402d7b
Replace cb_warning_x by cb_warning and mode unit test
sbelondr Jul 21, 2023
bb2edcf
Merge branch 'gcos4gnucobol-3.x' into gcos4gnucobol-3.x
GitMensch Jul 21, 2023
86c8cac
add check in syn_reportwriter.at and move warning in the typeck.c file
sbelondr Sep 7, 2023
15ecc19
Update ChangeLog
GitMensch Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cobc/ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@

2023-03-14 Samuel Belondrade <[email protected]>

* codegen.c (output_report_control): add a loop to check if the variable
exist (bug: #863)

2023-02-21 Simon Sobisch <[email protected]>

* codegen.c, flag.def [COBC_HAS_CUTOFF_FLAG]: fix compile errors,
Expand Down
25 changes: 23 additions & 2 deletions cobc/codegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9681,6 +9681,25 @@ output_report_control (struct cb_report *p, int id, cb_tree ctl, cb_tree nx)
if(nx) {
output_report_control(p, id, nx, CB_CHAIN(nx));
}

bfound = 0;
for(i= p->num_lines-1; i >= 0; i--) {
if(p->line_ids[i]->report_control) {
struct cb_field *c = cb_code_field (p->line_ids[i]->report_control);
if(c == s) {
bfound = 1;
break;
}
}
}
if (!bfound) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I moved this

cb_warning_x (COBC_WARN_FILLER,
CB_TREE(ctl), _("Control field %s is not referenced in report"), s->name);
ctl = NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

p->controls = NULL;
return ;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

output_local("/* Report %s: CONTROL %s */\n",p->name,s->name);
prvid = 0;
for(i = 0; i < p->num_lines; i++) {
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

output_local(",NULL");
}
seq = i = 0;
Expand Down Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

&& 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"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

(f->report_flag & COB_REPORT_LINE_PLUS)?"PLUS":"",
f->report_line);
f->report_line = 0;
Expand Down
46 changes: 46 additions & 0 deletions tests/testsuite.src/run_reportwriter.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9626,3 +9626,49 @@ BEFORE FINAL - SHOULD DISPLAY

AT_CLEANUP

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

AT_KEYWORDS([report])

AT_DATA([prog.cob], [
IDENTIFICATION DIVISION.
PROGRAM-ID. MAIN.
ENVIRONMENT DIVISION.
CONFIGURATION SECTION.
INPUT-OUTPUT SECTION.
FILE-CONTROL.
SELECT A_1 ASSIGN TO EXTERNAL A_1.
SELECT A_2 ASSIGN TO EXTERNAL A_2.
DATA DIVISION.
FILE SECTION.
FD A_1.
COPY test.
FD A_2 REPORT ETAT.
WORKING-STORAGE SECTION.
REPORT SECTION.
RD ETAT
CONTROL FINAL ERROR-1 ERROR-2
PAGE LIMIT 66.
PROCEDURE DIVISION.
STOP RUN.
])

AT_DATA([test.cpy], [
01 TEST-01.
03 TEST-03.
88 TEST-88 VALUE "0000000".
09 ERROR-1.
88 E-TEST-1 VALUE "111".
10 E-TEST-10 PIC 9(3).
03 E-TEST-03.
04 E-TEST-04.
05 FILLER PIC X(21).
88 E-TEST-2 VALUE " SM".
03 E-TEST-2-03.
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

prog.cob:18: warning: Control field ERROR-2 is not referenced in report
])

AT_CLEANUP