gnucobol icon indicating copy to clipboard operation
gnucobol copied to clipboard

Add --fdump-tree=FILE option

Open lefessan opened this issue 2 years ago • 8 comments

Dump the AST/tree to a file, for debugging purpose.

A file dump_ast_gen.c is generated by an external tool directly from tree.h, it can then be edited manually if the types are modified, until the tool is run again later to clean it up.

lefessan avatar Jul 14 '23 23:07 lefessan

Codecov Report

Merging #108 (00faae3) into gcos4gnucobol-3.x (6b44051) will decrease coverage by 1.38%. The diff coverage is 0.70%.

:exclamation: Current head 00faae3 differs from pull request most recent head 41819d1. Consider uploading reports for the commit 41819d1 to get more accurate results

:exclamation: 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     #108      +/-   ##
=====================================================
- Coverage              65.39%   64.02%   -1.38%     
=====================================================
  Files                     32       33       +1     
  Lines                  58797    60063    +1266     
  Branches               15492    16176     +684     
=====================================================
+ Hits                   38449    38454       +5     
- Misses                 14362    15621    +1259     
- Partials                5986     5988       +2     
Impacted Files Coverage Δ
cobc/codegen.c 75.33% <ø> (ø)
cobc/dump_tree.c 0.00% <0.00%> (ø)
cobc/parser.y 68.55% <0.00%> (ø)
cobc/cobc.c 72.02% <6.25%> (-0.29%) :arrow_down:
cobc/flag.def 100.00% <100.00%> (ø)
cobc/tree.c 74.09% <100.00%> (+0.01%) :arrow_up:
cobc/typeck.c 64.81% <100.00%> (ø)

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

codecov-commenter avatar Jul 14 '23 23:07 codecov-commenter

Would it be reasonable to use -fdump-something similar to GCC https://gcc.gnu.org/onlinedocs/gcc/Developer-Options.html ? Note: I don't have any idea if there are options that output JSON.

Question: What is the OCAML ml and where can I read more about it? What would this output be used for?

In any case please provide test cases with sample output (and to be sure that this doesn't break anything execute make checkall at least once passing this option in COBOL_FLAGS.

GitMensch avatar Jul 15 '23 11:07 GitMensch

I changed the options to -fdump-tree and -fdump-tree-flags, it should be close enough gcc devs expectations.

I ran all the testsuite with both formats (using env variables to not modify scripts), and I could validate both OCaml and JSON files.

I am not sure that adding a test in the testsuite would be a good idea: (1) the output depends on the internal types in tree.h, and (2) even for small examples, the output is pretty big, a few thousands lines...

The OCaml format is actually standard syntax for basic types in the OCaml language (https://ocaml.org). Since I mostly always program in OCaml (except for GnuCOBOL :-) ), it is easier for me to have this format than JSON, as I could easily write tools to simplify/filter information in the AST.

For examples, JSON format (with flags +Ai):

{
   "type_": "cb_program",
   "address_": " 0x559ec2ee75f8",
   "common": {
      "tag": { "type_": "cb_tag", "value_": "PROGRAM" },
      "source_file": "test.cob"
   },
   "program_name": "DOUBLE-OPEN",
   "program_id": "DOUBLE__OPEN",
   "orig_program_id": "DOUBLE-OPEN",
   "word_table": { "type_": "TODO", "value_": "struct cb_word **" },
   "local_include": { "type_": "TODO", "value_": "struct local_filename*" },
   "entry_list": [
      {
         "type_": "cb_list",
         "address_": " 0x559ec2f45138",
         "purpose": {
            "type_": "cb_label",
            "address_": " 0x559ec2f44f78",
            "common": {
               "tag": { "type_": "cb_tag", "value_": "LABEL" },
               "source_file": "test.cob",
               "source_line": 39
            },
            "name": "DOUBLE__OPEN",
            "orig_name": "DOUBLE-OPEN",
            "xref": { "type_": "TODO", "value_": "struct cb_xref" },
...

whereas OCaml format looks like:

RECORD [
   "type_", STRING "cb_program";
   "address_", POINTER 0x55acb46095f8L;
   "common", RECORD [
      "tag", CONSTR ("cb_tag", STRING "PROGRAM");
      "source_file", STRING "test.cob";
   ];
   "program_name", STRING "DOUBLE-OPEN";
   "program_id", STRING "DOUBLE__OPEN";
   "orig_program_id", STRING "DOUBLE-OPEN";
   "word_table", CONSTR ("TODO", STRING "struct cb_word **");
   "local_include", CONSTR ("TODO", STRING "struct local_filename*");
   "entry_list", LIST [
      RECORD [
         "type_", STRING "cb_list";
         "address_", POINTER 0x55acb4667138L;
         "purpose", RECORD [
            "type_", STRING "cb_label";
            "address_", POINTER 0x55acb4666f78L;
            "common", RECORD [
               "tag", CONSTR ("cb_tag", STRING "LABEL");
               "source_file", STRING "test.cob";
               "source_line", INT( 39);
            ];
            "name", STRING "DOUBLE__OPEN";
            "orig_name", STRING "DOUBLE-OPEN";
            "xref", CONSTR ("TODO", STRING "struct cb_xref");
...

lefessan avatar Jul 15 '23 21:07 lefessan

Note that there are still some TODOs in the code. There are many fields in these data structures, and it's not clear what should be printed or not.

Also, it might be interesting at some point to have "levels" of importance for fields, so that, for example, only important fields would be printed at level 1, then less important fields at level 2, and so on... However, such levels should probably be decided after experimenting with using these outputs for real debugging.

lefessan avatar Jul 15 '23 22:07 lefessan

That's strange, if you keep it inserted "down where it is" until there are possibly more changes, then there should be no conflict as no other commits as to the changelog down.

GitMensch avatar Oct 24 '24 04:10 GitMensch

That's strange, if you keep it inserted "down where it is" until there are possibly more changes, then there should be no conflict as no other commits as to the changelog down.

I see. For me, it has to be inserted at the point where it is merged, i.e. the ChangeLog before should correspond to what is in the code at the point where the work is done, and not when it was started, no ?

lefessan avatar Oct 24 '24 07:10 lefessan

That depends. If most of the old work is unchanged and "old", then I personally would have one "old" and one "new" entry. If it is totally new like this I'd likely move the Changelog entry up - but only directly before the actual merge (so no conflict in between).

GitMensch avatar Oct 24 '24 09:10 GitMensch

Last set of errors from the CI:

In file included from ../../cobc/dump_ast.c:190:
../../cobc/dump_ast_gen.c: In function 'init_ptr_table':
../../cobc/dump_ast_gen.c:39:5: error: implicit declaration of function 'bzero' [-Wimplicit-function-declaration]
   39 |     bzero (ptr_table, ptr_table_size * sizeof(void*) );
mv -f .deps/replace.Tpo .deps/replace.Po
      |     ^~~~~
../../cobc/dump_ast_gen.c:39:5: warning: incompatible implicit declaration of built-in function 'bzero' [-Wbuiltin-declaration-mismatch]
../../cobc/dump_ast_gen.c: In function 'print_struct_cb_funcall':
../../cobc/dump_ast_gen.c:7826:40: warning: the comparison will always evaluate as 'true' for the address of 'argv' will never be NULL [-Waddress]
 7826 |     if( print_zero_fields || ptr->argv != NULL ){
      |                                        ^~
In file included from ../../cobc/dump_ast.c:37:
../../cobc/tree.h:1288:33: note: 'argv' declared here
 1288 |         cb_tree                 argv[CB_BUILD_FUNCALL_MAX];     /* Function arguments */
      |                                 ^~~~
../../cobc/dump_ast_gen.c: In function 'print_struct_cb_intrinsic':
../../cobc/dump_ast_gen.c:8420:58: warning: passing argument 2 of 'print_struct_cb_intrinsic_table_ptr' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
 8420 |        print_struct_cb_intrinsic_table_ptr (indent+2, ptr->intr_tab);
      |                                                       ~~~^~~~~~~~~~
../../cobc/dump_ast_gen.c:7995:89: note: expected 'struct cb_intrinsic_table *' but argument is of type 'const struct cb_intrinsic_table *'
 7995 | static void print_struct_cb_intrinsic_table_ptr (int indent, struct cb_intrinsic_table *ptr){
      |                                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
../../cobc/dump_ast.c: In function 'cb_dump_ast_to_file':
../../cobc/dump_ast.c:240:21: warning: unused variable 'len' [-Wunused-variable]
  240 |                 int len = strlen (filename);
      |                     ^~~
../../cobc/dump_ast_gen.c: At top level:
../../cobc/dump_ast_gen.c:2509:13: warning: 'print_struct_cob_prof_procedure_ptr' defined but not used [-Wunused-function]
 2509 | static void print_struct_cob_prof_procedure_ptr (int indent, struct cob_prof_procedure *ptr){
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../cobc/dump_ast_gen.c:2090:13: warning: 'print_struct_cb_call_xref_ptr' defined but not used [-Wunused-function]
 2090 | static void print_struct_cb_call_xref_ptr (int indent, struct cb_call_xref *ptr){
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../cobc/dump_ast_gen.c:2036:13: warning: 'print_struct_handler_struct_ptr' defined but not used [-Wunused-function]
 2036 | static void print_struct_handler_struct_ptr (int indent, struct handler_struct *ptr){
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../cobc/dump_ast_gen.c:1258:13: warning: 'print_struct_cb_xref_ptr' defined but not used [-Wunused-function]
 1258 | static void print_struct_cb_xref_ptr (int indent, struct cb_xref *ptr){
      |             ^~~~~~~~~~~~~~~~~~~~~~~~
../../cobc/dump_ast.c:162:13: warning: 'print_int_array' defined but not used [-Wunused-function]
  162 | static void print_int_array (int indent, int n, int *x)
      |             ^~~~~~~~~~~~~~~
make[3]: *** [Makefile:639: dump_ast.o] Error 1

The MSVC builds all fail because the new dump_ast files need to be added to build_windows.

GitMensch avatar Oct 24 '24 20:10 GitMensch