Initial addition of CONSTANT SECTION by RamyGeorge · Pull Request #278 · OCamlPro/gnucobol

RamyGeorge · 2026-03-03T17:58:36Z

Summary

This pull request introduces the CONSTANT SECTION as defined for Fujitsu Language reference. Its intended purpose is to introduce VALUES that do not change during program execution. For now, It expects record description entries.

Changes

Addition of the grammar for CONSTANT SECTION
New Flag for COBC_HD_CONSTANT_SECTION

Motivation

Specific section for adding CONSTANT values while enforcing proper syntax

Tests

Test for the grammar is added at tests/testsuite.src/syn_definition.at
Test for MOVE with a CONSTANT is expected to fail for now

Please give any feedback and/or any changes required to fulfill current PR goal.

GitMensch

That's a good first take!
Please add cobc/Changelog to your changes as well-

please see https://sourceforge.net/p/gnucobol/wiki/Style%20guide/ for some general hints

The test currently fail (run make check and after the first one make check TESTSUITEFLAGS=--recheck --verbose locally to see them) because of

-prog.cob:6: error: Items in CONSTANT SECTION must have VALUE
+prog.cob:2: note: free format detected
+prog.cob:7: error: Items in CONSTANT SECTION must have  VALUE 
+prog.cob:7: warning: handling of CONSTANT SECTION code generation is unfinished; implementation is likely to be changed

for line two: start DIVISIONs and SECTIONs and paragraphs at column 8 and each "real" code at column 12; line 7 will be line 6 after the suggested change

GitMensch · 2026-03-03T18:45:53Z

cobc/field.c

+	if(f->storage == CB_STORAGE_CONSTANT){
+		if(!f->values){
+			cb_error(_("Items in CONSTANT SECTION must have  VALUE "));


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)

GitMensch · 2026-03-03T18:46:54Z

cobc/field.c

 	}

+	if(f->storage == CB_STORAGE_CONSTANT){
+		if(!f->values){


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

GitMensch · 2026-03-03T18:50:03Z

cobc/parser.y

 	COBC_HD_SCREEN_SECTION		= (1U << 17),
-	COBC_HD_PROCEDURE_DIVISION	= (1U << 18)
+	COBC_HD_PROCEDURE_DIVISION	= (1U << 18),
+	COBC_HD_CONSTANT_SECTION = (1U << 19) 


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

GitMensch · 2026-03-03T18:56:41Z

cobc/parser.y

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


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)

GitMensch · 2026-03-03T19:07:15Z

cobc/field.c


+	if(f->storage == CB_STORAGE_CONSTANT){
+		if(!f->values){
+			cb_error(_("Items in CONSTANT SECTION must have  VALUE "));


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

GitMensch · 2026-03-03T19:09:28Z

cobc/field.c

+			cb_error(_("Items in CONSTANT SECTION must have  VALUE "));
+		}
+
+		CB_UNFINISHED ("CONSTANT SECTION code generation");


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

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)

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

GitMensch · 2026-03-03T19:09:36Z

cobc/parser.y

 	cb_tree x;
 	const int level = cb_get_level ($1);
-
+	


trim trailing whitespace

GitMensch · 2026-03-03T19:09:41Z

cobc/tree.h

 	struct cb_field		*linkage_storage;	/* LINKAGE */
 	struct cb_field		*screen_storage;	/* SCREEN */
 	struct cb_field		*report_storage;	/* REPORT */
+	struct cb_field     *constant_storage;  /* CONSTANT */


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)

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

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

GitMensch · 2026-03-03T19:09:46Z

tests/testsuite.src/syn_definition.at

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


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

GitMensch · 2026-03-03T19:09:48Z

tests/testsuite.src/syn_move.at

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


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

GitMensch · 2026-03-03T19:17:57Z

follow-up additions after the mentioned issues:

have a test that MOVEs a constant to a field, then DISPLAYs that field
have a look how level 78 constants define their constness in their definition - using the same or similar may solve the MOVE TO CONSTANT issue
optional: in codegen possibly use the constant prefix (which is used for fields built from literals) instead of the field one
ideally: in codegen generate the data in the same way the constant data (which is used for string literals) is setup
follow-on:
- add a new scope "CO" to -fdump option that handles the elements in the constant_storage, along with a test
- internally set all level 78 constants and 01 cosntants defined in any storage into the constant_storage (so they will dump as well)

GitMensch

mostly testing issues in this review iteration;

please have a look at the already specified changes to -fdump and add a new test to listings.at with -ftsymbol (just edit an existing test and add a CONSTANT SECTION into it, then see if the symbol listing includes those or not

GitMensch · 2026-03-06T12:06:14Z

cobc/ChangeLog

@@ -1,3 +1,11 @@
+2026-03-03  Ramy George <ramygeorge19@gmail.com>


please start with a newline (that helps with a bunch of diff algrothms for some reason)

GitMensch · 2026-03-06T12:06:57Z

tests/testsuite.src/syn_definition.at

+       CONSTANT SECTION.
+       01  NUM-CONST   PIC 9(3).
+       01  OTHER-CONST PIC 9(3) VALUE 123.
+       PROCEDURE DIVISION.


please add a third constant with a constant record

GitMensch · 2026-03-06T12:09:43Z

tests/testsuite.src/syn_move.at

+           STOP RUN.
+])
+
+AT_CHECK([$COMPILE prog.cob], [1], [],[ignore])


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

GitMensch · 2026-03-06T12:14:33Z

tests/testsuite.src/syn_move.at

+AT_SETUP([MOVE CONSTANT SECTION to field])
+AT_KEYWORDS([extension constant move])
+
+AT_XFAIL_IF([true]) 
+
+AT_DATA([prog.cob],[
+       IDENTIFICATION DIVISION.
+       PROGRAM-ID. prog.
+       DATA DIVISION.
+       CONSTANT SECTION.
+       01  MY-CONST    PIC 9(3) VALUE 123.
+       WORKING-STORAGE SECTION.
+       01  RESULT-FLD  PIC 9(3).
+       PROCEDURE DIVISION.
+           MOVE MY-CONST TO RESULT-FLD.
+           DISPLAY RESULT-FLD.
+           STOP RUN.
+])
+# This is also not implemented yet but should help towards the bigger issue with CONSTANTs being replaced by literals
+AT_CHECK([$COMPILE prog.cob], [0], [], [])
+AT_CLEANUP


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.

GitMensch · 2026-03-06T12:16:57Z

cobc/typeck.c

 	switch (storage) {
 	case CB_STORAGE_CONSTANT:
-		return "Constants";
+		return "CONSTANT SECTION";


Constants may should be lower-cased (not sure), but CONSTANT SECTION is wrong here as other constants have the same storage but are not "living" in that section

Initial addition of CONSTANT SECTION

ebb3b05

GitMensch requested changes Mar 3, 2026

View reviewed changes

Corrections and follow up on previous CONSTANT SECTION changes

7c8d4bb

GitMensch requested changes Mar 6, 2026

View reviewed changes

		@@ -1,3 +1,11 @@
		2026-03-03 Ramy George <ramygeorge19@gmail.com>

Conversation

RamyGeorge commented Mar 3, 2026

Summary

Changes

Motivation

Tests

Uh oh!

GitMensch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GitMensch commented Mar 3, 2026

Uh oh!

GitMensch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants