fortran_function_parser icon indicating copy to clipboard operation
fortran_function_parser copied to clipboard

Problems with Pre-Processor Directives

Open kthoe opened this issue 3 years ago • 4 comments

Some compilers (e.g. GCC 9) don't much like the preprocessor directives, and other compilers (e.g. Intel ifort 18) are only conditionally fine.

There are two issues:

  1. #ifdef REAL32; fparser_rk = real32 This could be fragile if the preprocessor is case insensitive (i.e. fparser_rk = REAL32 instead), which might lead to "recursion detected" compilation errors. It might be better to use something along the lines of #ifdef SINGLE or #ifdef PARSER_32 instead.
  2. #ifdef REAL32; #elif REAL64 is fragile. Some compilers are conditionally fine if REAL32 has a value (e.g. #define REAL32 REAL32 instead of just #define REAL32). Other compilers don't really like that structure at all. It'd be more broadly compatible to do something like #if defined(REAL32); #elif defined(REAL64) instead.

kthoe avatar Jun 15 '22 17:06 kthoe

  1. Is there a compiler and platform where this happens? I'm using this on many other projects, and have never seen any issues. (e.g., JSON-Fortran is using the same thing and is tested with Gfortran 7, 8, 9, and 10).
  2. I don't fully understand this. The intent is to pass -DREAL32 to the compiler to enable single precision. Are you saying there are instances where this doesn't work?

jacobwilliams avatar Jun 17 '22 00:06 jacobwilliams

For specificity, using GCC 9.4.0, something like:

#define REAL64
#include "function_parser.f90"
#undef REAL64

Results in the error:

 #elif REAL64
Error: #elif with no expression

Using an #if defined(REAL32) and #elif defined(REAL64) structure instead solves that issue.

If the code were modified to be integer,parameter,public :: fparser_rk = REAL32 instead of the current integer,parameter,public :: fparser_rk = real32 then the value of REAL32 would be substituted, which can result in a variety of problems or unexpected behaviors. Basically the code as it sits is case sensitive, clashing with the Fortran standard.

kthoe avatar Jun 17 '22 01:06 kthoe

But why are you including the module in another file? That's not really the intended use case.

jacobwilliams avatar Jun 17 '22 01:06 jacobwilliams

That was more intended to be illustrative, but including it in a file could be a good way to support both single and double precision parsing in a polymorphic context.

kthoe avatar Jun 18 '22 01:06 kthoe