OCamlPro / gnucobol

A clone of the sourceforge GnuCOBOL compiler from COBOL to C.
https://get-superbol.com
GNU Lesser General Public License v3.0
16 stars 20 forks source link

New option --copy COPYBOOK, to include a COPYBOOK #114

Open lefessan opened 9 months ago

lefessan commented 9 months ago

This option provides the same behavior as --include FILE for gcc and clang, i.e. including a file (or a group of files) before parsing the source. It can be used typically to include REPLACE statements.

codecov-commenter commented 9 months ago

Codecov Report

Merging #114 (993e3a4) into gcos4gnucobol-3.x (c0d64ad) will increase coverage by 0.02%. The diff coverage is 86.53%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x     #114      +/-   ##
=====================================================
+ Coverage              65.74%   65.76%   +0.02%     
=====================================================
  Files                     32       32              
  Lines                  59092    59133      +41     
  Branches               15575    15587      +12     
=====================================================
+ Hits                   38849    38890      +41     
+ Misses                 14262    14259       -3     
- Partials                5981     5984       +3     
Files Coverage Δ
cobc/flag.def 100.00% <100.00%> (ø)
cobc/help.c 100.00% <100.00%> (ø)
cobc/replace.c 96.20% <ø> (+0.94%) :arrow_up:
cobc/cobc.c 72.51% <91.66%> (+0.12%) :arrow_up:
cobc/codegen.c 75.98% <60.00%> (-0.02%) :arrow_down:
cobc/pplex.l 72.67% <85.00%> (+0.05%) :arrow_up:

... and 2 files with indirect coverage changes

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

GitMensch commented 9 months ago

As I've just recognized that: this could also serve for some people to gather compile options "the other way" using a file with directives, which are then included by this option.

It would also provide a "similar" option to MicroFocus -C DIRECTIVES "FILE" (take the old file, copy as ".cpy" go in there and add $SET in front of each line, then compile in cobc with --copy your.dir.cpy.

GitMensch commented 9 months ago

Hm, and another one - mostly a question @lefessan: how does this apply to -fformat=auto, or better to say: shouldn't the auto-format be "reset" after each of the included --copy elements?

lefessan commented 9 months ago

It would also provide a "similar" option to MicroFocus -C DIRECTIVES "FILE" (take the old file, copy as ".cpy" go in there and add $SET in front of each line, then compile in cobc with --copy your.dir.cpy.

It should probably be part of a document of recipes to migrate from MicroFocus to GnuCOBOL. Unfortunately, the localisation of errors would make it difficult to debug in case of problem, it would be better to call a directive entry point in the parser for each directives in a list, juste before including the copies.

GitMensch commented 9 months ago

it would be better to call a directive entry point in the parser for each directives in a list, just before including the copies

That would be interesting, especially if it handles the different directive variants. Something to keep in mind for later.

lefessan commented 9 months ago

Hm, and another one - mostly a question @lefessan: how does this apply to -fformat=auto, or better to say: shouldn't the auto-format be "reset" after each of the included --copy elements?

Good point. I just checked: if the format is auto, the detection is performed on the main target, the format is then set, and all copybooks are expected to be of the same format. I think it's a reasonable behavior.

GitMensch commented 9 months ago

I agree, that's a reasonable approach, possibly also to suggest users to use format-independent COBOL in those --copy included copybooks ;-)

GitMensch commented 9 months ago

As these are new options: please add them to the NEWS. I suggest to do that here already to not forget it on the later commit.

lefessan commented 9 months ago

I just added the NEWS section.

I am still not comfortable with using absolute paths with --include. Recently, we discussed about using Docker to provide simpler multi-platform installations for GnuCOBOL: in my experience, it works pretty well, but only if all your paths are relative (the idea is to mount the local directory somewhere and run the program inside the container in that directory, so that it can access all local files). So, the --include "$PWD/FILE.h" will fail in such a context, since the absolute path is not the one in the Docker image.

If we cannot make a relative path the default, I added a commit to add a COBC_BUILD_INPLACE env variable that makes the C file to be generated and build in the local directory. It's not my preferred solution, but it would allow the Docker version to work correctly by setting it inside and outside the container.

GitMensch commented 9 months ago

Can you please explain where you see the problem with something like --include "FILE.h" -I $PWD or --include "FILE.h" -g or --include "FILE.h" --save-temps?

lefessan commented 9 months ago

I didn't notice that -g could be used for that.

For me, the behavior of --include FILE.h should not depend on whether it is used with or without another unrelated option. Indeed, --include FILE.h -g will probably be the set of options used by most users, so it will work without $PWD, but then, they will not understand why removing -g suddenly make their build fail.

For now, I only see scenarios where the option would only be used in development mode (so, with -g), but my experience is that one day or another, somebody will find a way to try to use this option in an unexpected way in release mode too (to replace some calls by macros ?), so we should let the door opened...

GitMensch commented 9 months ago

so we should let the door opened...

We do with --save-temps or -I $TMPDIR. You may consider checking gnucobol.tex if those three possibilities are mentioned there or should be mentioned. I'm not sure we need that at all, because, as I've said, people most commonly use that option with including a file from the system include directories; for me a single half sentence about this "note that to find the file you may need to pass an appropriate -I" or alike would be enough.

GitMensch commented 9 months ago

What's the benefit of COBC_BUILD_INPLACE over --save-temps? I guess the difference is it would place the intermediate files locally, but then deletes them afterwards, right?

lefessan commented 9 months ago

Yes, it does not leave intermediate files in the local dir. (FWIW, my latency is high since last week as I am in a two-week vacation with my kids... and I didn't have as much time as I expected to hack on my spare time :-) )

lefessan commented 6 months ago

Please drop COBC_BUILD_INPLACE and replace as noted in the previous posts.

I am still not fond of using --save-temps to get this behavior, as it will pollute the local directory with unwanted intermediate files. Is it possible to keep it ?

GitMensch commented 6 months ago

Isn't it enough to have -I . if you want the local path?

In any case: #112 can be integrated upstream before.

lefessan commented 6 months ago

Merged.

GitMensch commented 6 months ago

Thank you. I've just recognized that there's one missing part from FR # 176: Back in the discussions then the point was mot sources that call into C always need the same C header, and therefore a new directive was the goal.

Can you or @engboris have a look at adding the >>IMP INCLUDE directive and close the related FR by that?

GitMensch commented 4 months ago

reopen for the related missing part of the FR

engboris commented 3 months ago

Thank you. I've just recognized that there's one missing part from FR # 176: Back in the discussions then the point was mot sources that call into C always need the same C header, and therefore a new directive was the goal.

Can you or @engboris have a look at adding the >>IMP INCLUDE directive and close the related FR by that?

I'm currently working on it. What should happen if both the compiler option and the directive are defined? Should one be ignored? Both be considered?

I also plan to allow appending on text_list (probably restricted to cb_include_file_list) in cobc/cobc.h because it doesn't look like I'm able to do it from ppparse.y where the directive is parsed and defined.

EDIT : just noticed there are functions for that for the second paragraph of my answer

GitMensch commented 3 months ago

What should happen if both the compiler option and the directive are defined?

You may also have multiple >> IMP INCLUDE - each one (as well as the command line option) add to the list, the list is restored before the next input source file is handled (similar to source format).