Skip to content

Initial addition of CONSTANT SECTION #278

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
RamyGeorge wants to merge 2 commits into
base: gitside-gnucobol-3.x
Choose a base branch
from
Open

Initial addition of CONSTANT SECTION #278

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ebb3b05
Initial addition of CONSTANT SECTION
RamyGeorge Mar 3, 2026
7c8d4bb
Corrections and follow up on previous CONSTANT SECTION changes
RamyGeorge Mar 5, 2026
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
10 changes: 9 additions & 1 deletion cobc/field.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3408,11 +3408,19 @@ cb_validate_field (struct cb_field *f)
f->flag_invalid = 1;
return;
}
if (f->flag_item_78) {
if (f->flag_item_78) {
f->flag_is_verified = 1;
return;
}

if(f->storage == CB_STORAGE_CONSTANT){
if(!f->values){
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

I'd have to check the rules but it could be that for a record structure only part of it needs a VALUE clause - I really don't know (the Fuji manual should have the rules in).

cb_error(_("Items in CONSTANT SECTION must have VALUE "));
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

formatting: see whitespace as used in other places (and drop the additional trailing spaces in line 3411, please)

msgid (that's hard to get right from the start, you only can get a clue how it might look like when checking the others) should possibly be "item in CONSTANT SECTION must have a VALUE clause" (again no trailing space)

Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

you want cb_error_x (CB_TREE(f), .... here as the current line number is already after the field and you want its starting position

}

CB_UNFINISHED ("CONSTANT SECTION code generation");
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

that would come for every line (please add a second var, maybe a record without VALUE to the test to highlight it) - most easy solution would possibly be to add it on the program validation level if constant_section is non-NULL

Copy link
Author

@RamyGeorge RamyGeorge Mar 5, 2026

Choose a reason for hiding this comment

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

The only place I did find such validation is cb_validate_program_data. I am not exactly sure where to put it within all the logic but i have added a check for cb_storage_constant at the end and called for CB_UNFINISHED)

Copy link
Collaborator

@GitMensch GitMensch Mar 5, 2026

Choose a reason for hiding this comment

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

yes similar, but instead of the constant you'd check the program's ->constant_storage (once it gets any addition, I don't think the constant section elements currently do get linked there, but into the previous storage).

}

/* Set up parameters */
if (f->storage == CB_STORAGE_LOCAL ||
f->storage == CB_STORAGE_LINKAGE ||
Expand Down
21 changes: 19 additions & 2 deletions cobc/parser.y
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ enum cobc_hd {
COBC_HD_LINKAGE_SECTION = (1U << 15),
COBC_HD_REPORT_SECTION = (1U << 16),
COBC_HD_SCREEN_SECTION = (1U << 17),
COBC_HD_PROCEDURE_DIVISION = (1U << 18)
COBC_HD_PROCEDURE_DIVISION = (1U << 18),
COBC_HD_CONSTANT_SECTION = (1U << 19)
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

this should have the right order to be able to use > / >= operators;
per Fuji manual CONSTANT SECTION goes before LINKAGE (As you did with the parser later on

};

/* Static functions */
Expand Down Expand Up @@ -6437,6 +6438,7 @@ _data_division:
_working_storage_section
_communication_section
_local_storage_section
_constant_section
_linkage_section
_report_section
_screen_section
Expand Down Expand Up @@ -7403,7 +7405,7 @@ constant_entry:
{
cb_tree x;
const int level = cb_get_level ($1);

Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

trim trailing whitespace

cobc_cs_check = 0;
if (level != 1) {
cb_error (_("CONSTANT item not at 01 level"));
Expand Down Expand Up @@ -9077,6 +9079,21 @@ _local_storage_section:
}
;

/* CONSTANT SECTION */

constant_section: CONSTANT {check_area_a_of("CONSTANT SECTION");};
_constant_section:

| constant_section SECTION _dot
{
check_headers_present(COBC_HD_DATA_DIVISION,0,0,0); // check for Data division before hand
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

the comment is correct but superfluous here, note that per style guide: we only use C89 comment blocks (as GC still compiles on C89 compilers that don't have // inline commments)

header_check |= COBC_HD_CONSTANT_SECTION;
current_storage = CB_STORAGE_CONSTANT;
}
_record_description_list
;



/* LINKAGE SECTION */

Expand Down
1 change: 1 addition & 0 deletions cobc/tree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,7 @@ struct cb_program {
struct cb_field *linkage_storage; /* LINKAGE */
struct cb_field *screen_storage; /* SCREEN */
struct cb_field *report_storage; /* REPORT */
struct cb_field *constant_storage; /* CONSTANT */
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

that is the right thing to add - but you'd have to add the fields to that as well and then "read" the fields from there later on (have a look how that is done with working_storage)

Copy link
Author

@RamyGeorge RamyGeorge Mar 5, 2026

Choose a reason for hiding this comment

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

This I'm not exactly sure what should be added. I thought this is mainly for building the AST so I'm not particularly sure what is missing

Copy link
Collaborator

@GitMensch GitMensch Mar 5, 2026

Choose a reason for hiding this comment

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

When producing AST we have a "current storage" where fields are added to (a linked list of 01/77 fields) - I think while the current_storage is setup correctly, the assignment to add fields to that list isn't.
For other storage elements those are already used for the symbol printing (have a look at code for -tsymbols - it actually should be added there) and at the end for the codegen.
The new storage will also be added (but just for the -fdump part) into codegen by starting from that linked list.

When you add the new storage to those two places, you'll likely see that they are "empty".

cb_tree local_file_list; /* Local files */
cb_tree global_file_list; /* Global files */
struct handler_struct global_handler[5]; /* Global handlers */
Expand Down
20 changes: 20 additions & 0 deletions tests/testsuite.src/syn_definition.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2947,3 +2947,23 @@ AT_DATA([prog.cob], [

AT_CHECK([$COMPILE_ONLY -Wno-unfinished prog.cob], [0], [], [])
AT_CLEANUP


# CONSTANT SECTION
AT_SETUP([CONSTANT SECTION syntax])
AT_KEYWORDS([extension constant VALUE])

AT_DATA([prog.cob],[
IDENTIFICATION DIVISION.
PROGRAM-ID. TEST-NO-VALUE.
DATA DIVISION.
CONSTANT SECTION.
01 NUM-CONST PIC 9(3).
PROCEDURE DIVISION.
STOP RUN.

])

AT_CHECK([$COMPILE_ONLY prog.cob], [1], [],[prog.cob:6: error: Items in CONSTANT SECTION must have VALUE
])
AT_CLEANUP
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

that wants a line break at the end; please also have a look at how the other tests are formatted (only slightly different)

22 changes: 22 additions & 0 deletions tests/testsuite.src/syn_move.at
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,3 +752,25 @@ prog.cob:7: note: 'BIGFLT' defined here as USAGE FLOAT

AT_CLEANUP



AT_SETUP([MOVE to CONSTANT SECTION item])
AT_KEYWORDS([extension constant move])

AT_XFAIL_IF([true])
Copy link
Collaborator

@GitMensch GitMensch Mar 3, 2026

Choose a reason for hiding this comment

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

add a comment why that is an expected fail ("to be implemented soon" :-)

each new AT_SETUP gets two empty lines above, so please drop one


AT_DATA([prog.cob],[
IDENTIFICATION DIVISION.
PROGRAM-ID. prog.
DATA DIVISION.
CONSTANT SECTION.
01 NUM-CONST PIC 9(3) VALUE 123.
PROCEDURE DIVISION.
MOVE 456 TO NUM-CONST
STOP RUN.
])

AT_CHECK([$COMPILE prog.cob], [1], [],[ignore])
Copy link
Collaborator

@GitMensch GitMensch Mar 6, 2026

Choose a reason for hiding this comment

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

you may add -Wno-unfinished to the command line - please add the expected error into stderr instead of ignore (see tests above how to split the AT_CHECK into multiple lines for that


AT_CLEANUP
Comment on lines +777 to +797
Copy link
Collaborator

@GitMensch GitMensch Mar 6, 2026

Choose a reason for hiding this comment

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

this test should be moved to run_extensions (with -Wno-unfinished in the compilation) and should include testing the actual result (which I guess will be fine);
please add another runtime test into run_extensions.at that has an alphanumeric constant and uses refererence modification [substring], like MOVE MY-CONST (2:2) TO RESULT-FLD. - I guess that will currently fail during compile.


Loading