zerothi / fdict

Fortran type-free variable and type-free dictionary
http://zerothi.github.io/fdict
Mozilla Public License 2.0
75 stars 17 forks source link

Using plain C preprocessor instead of fypp? #34

Closed LecrisUT closed 1 year ago

LecrisUT commented 1 year ago

What is the main advantage of using fypp over the built-in preprocessor (can be enabled by renaming f90 -> F90)? It would be nice to reduce the dependencies required for the library user, especially those that can not be automated with FetchContent.

zerothi commented 1 year ago

Thanks for your issues! :)

Let me take this one first. I just recently switched to fypp. This is mainly because of the complexity that is required for the cpp preprocessor. Simple things quickly gets complicated when doing c preprocessing, you can check commit older than 246ca7c162520e2efce6ee5aa56959776f518f71 which contains the header preprocessing.

Another thing was that the cpp preprocessor was not well defined. In fact gfortran preprocessing is a non-standard and each compiler are free to do whatever they wishes. This uncovered maintenance problems for various cpp executables which did different things. For instance ifort -E -P -xc would be the equivalent of gfortran -E -P -x c (note the space). This is an annoyance for end-users.

Lastly, fypp is shipped with the sources (not a submodule), so there should be no problems, in any circumstance. I'll close this soonish as I don't want to go back.

LecrisUT commented 1 year ago

Lastly, fypp is shipped with the sources (not a submodule), so there should be no problems, in any circumstance. I'll close this soonish as I don't want to go back.

Actually this was not picked up when using FetchContent. Also I would suggest to use FetchContent to download the appropriate version of fypp instead of update_fypp.sh. This also ensures downstream has control over what version of fypp to use.

For instance ifort -E -P -xc would be the equivalent of gfortran -E -P -x c (note the space).

I don't know what the -x flag does, but I would point out that cmake does handle the preprocessing automatically. I am working on octopus which has its own ridiculous use of pre-processor, but I did not have any issues porting it to cmake on neither gfortran, nor ifx. I notice that the commit you've pointed to there was no cmake toolchain.

zerothi commented 1 year ago

Lastly, fypp is shipped with the sources (not a submodule), so there should be no problems, in any circumstance. I'll close this soonish as I don't want to go back.

Actually this was not picked up when using FetchContent. Also I would suggest to use FetchContent to download the appropriate version of fypp instead of update_fypp.sh. This also ensures downstream has control over what version of fypp to use.

One should never be required to call update_fypp.sh, so again, I would not do anything like that. It simply isn't needed. What is weird to me is that your configuration doesn't automatically pick the path up.

For instance ifort -E -P -xc would be the equivalent of gfortran -E -P -x c (note the space).

I don't know what the -x flag does, but I would point out that cmake does handle the preprocessing automatically. I am working on octopus which has its own ridiculous use of pre-processor, but I did not have any issues porting it to cmake on neither gfortran, nor ifx. I notice that the commit you've pointed to there was no cmake toolchain.

Excatly, cmake entered a couple of weeks ago.

Yes, ideally, but I would still like to support the makefile for some time. Octopus had to do the same (other preprocessing) because there are limiting stuff in the C preprocessor that needs to be dealt with for fortran (using // in the c preprocessor is a pain :)) Suffice to say that the C preprocessor is meant for C/C++, and fypp is for fortran. Neither are perfect, but fypp makes life much easier. And there are plenty of packages that rely on fypp because it just works.

LecrisUT commented 1 year ago

Sure thing, if you've found it works much better I will trust you on this. It will be an uphill battle to convince the octopus folks to allow using python, which is the only reason for bringing this up. But if that is successful, there are so many custom tooling that I want to burn to the ground. Hopefully the usefulness of this package can spark that fuel.

But for this issue I would still suggest resolving the issues:

zerothi commented 1 year ago

Could you try the latest commit, it should fix the issue. No need for any of the complicated things.

LecrisUT commented 1 year ago

Could you try the latest commit, it should fix the issue. No need for any of the complicated things.

Confirmed recent commit fixes the find

zerothi commented 1 year ago

Great, I'll close now.