v2412 does not compile with clang in distributions with an old flex version

Summary

In c++17, the 'register' keyword has been removed. gcc still compiles code that includes it by issuing a warning but clang treats it as an error.

However, distributions with an old flex version (< 2.6.0) will produce .C files that include this keyword. This is what happens when parsing src/fileFormats/stl/STLAsciiParseFlex.L, eventually leading to compilation errors when using clang (version 15.0.4 for me, but I guess the issue will emerge with all c++17-enabled versions).

Though quite old, many clusters run Linux distributions that ship with flex versions < 2.6.0 (Amazon Linux 2 and RHEL 6.10 use 2.5.35, CentOS 7.9 uses flex 2.5.37). I ran into this issue in AWS, the Greek national supercomputer ARIS, and, of course our own cluster

Possible fixes

Maybe the installation scripts can give the option to install a recent version of flex (2.6.4 worked fine for me, when compiled manually in the above mentioned systems) in the ThirdParty folder. Then, the bin folder of the compiled flex version can be exported to $PATH. Additionally, the include folder of the compiled flex version should be added to the wmake rules, to allow the compiler to see the newer FlexLexer.h header.

assigned to @mark

Hi @vaggelisp

I am very reluctant to add yet another build script into third-party. Could consider adding a wmake/scripts/have_flex style of script (similar to the have_readline version). But then would need to inject flex path values back into the wmake rules. So still not really pretty.

Other possible solutions (hacks) - define register as __register__ (or nothing) for applications using flex.

Also, we can actually remove the flex usage for src/OpenFOAM, but never actually got it tested well enough to remove it. See ea71484e for the initial commit.

Hi @mark

I see. Compilation can already be quite complex with the existing dependencies.

Adding #define register to STLAsciiParseFlex.L indeed solves the compilation issue, but #define register __register__ still ends up with compilation errors. Since v2412 is already using c++17 and the register keyword is removed from this standard, this could indeed be a simple solution.

So this means this type of change?

// Flex may use register, which is deprecated and incompatible with C++17
#pragma clang diagnostic ignored "-Wdeprecated-register"
#undef  register
#define register  /* define as nothing to effectively remove from code as well (issue #3337) */

This is starting to look really ugly.

If you feel like applying to all flex files, testing and then giving back as patch it would definitely be helpful. Otherwise I'll have to try and find an old flex for testing. All of the flex files should have the clang diagnostic already, which makes them easy to find.

Here is the patch of applying the change you mentioned in all .L files (all compile without errors or warnings). These are 6 in total, namely

applications/utilities/mesh/conversion/ansysToFoam/ansysToFoam.L
applications/utilities/mesh/conversion/fluent3DMeshToFoam/fluent3DMeshToFoam.L
applications/utilities/mesh/conversion/fluentMeshToFoam/fluentMeshToFoam.L
applications/utilities/mesh/conversion/gambitToFoam/gambitToFoam.L
src/fileFormats/stl/STLAsciiParseFlex.L
src/thermophysicalModels/reactionThermo/chemistryReaders/chemkinReader/chemkinLexer.L

Since we are eliminating register, I guess the #pragma can be removed too (compiled with no warnings when doing so).

Agreed, this is not very nice, but none of the discussed solutions is pretty. Either an external flex version is provided and wmake rules are augmented or we follow the #define approach.

0001-BUG-files-using-flex-don-t-compile-with-clang-and-fl.patch

Will apply it tomorrow and push. No particular harm leaving in the #pragma for now...

mentioned in commit 47575aab

closed

added community label