Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement standard assert subroutine and associated macros #72

Open
rweed opened this issue Jan 3, 2020 · 12 comments
Open

Implement standard assert subroutine and associated macros #72

rweed opened this issue Jan 3, 2020 · 12 comments
Labels
topic: utilities containers, strings, files, OS/environment integration, unit testing, assertions, logging, ...

Comments

@rweed
Copy link

rweed commented Jan 3, 2020

Per @certiks request, I propose we extend his stdlib_experimental_error.f90 code to a standard assert subroutine and supplement it with some pre-processor macros. As an example here is my implementation of an assert routine and the associated macros

assertions.f90

assert.txt

The associated preprocessor macros are
assert_macros.txt

Note, these are my implementation of similar routine and macros found in the FTL project

@pdebuyl pdebuyl mentioned this issue Jan 3, 2020
@milancurcic
Copy link
Member

I think this is a good step forward. I planned to propose adding optional message to assert so that it can be used more universally for tests and validating user inputs, and this takes it even further with file, line, and optional abort on failure.

I suggest everything but the first argument to be optional so it can be used in its minimal capacity.

Minor nit-pick: assertion .NEQV. .TRUE. can be just .not. assertion.

@rweed
Copy link
Author

rweed commented Jan 3, 2020

My primary use for assert was in testing which is why I have the two global variables (num_failed_asserts, and num_asserts). Usually you have several tests you want to perform at one time and want some record of the number of failed tests versus the number of tests and additional information as to when/where the failed test occured. However, I tried to also allow for the case where you want to use assert for error_handling in a given subroutine etc and do a hard abort/stop when an appropriate stop condition was met. A couple of things with my code.

  1. IK=>INT32 should be appended to the end of the USE ISO_FORTRAN_ENV. I normally have a module that defines IK but I removed it for simplicity.

  2. There probably needs to be some kind of RESET_ASSERT subroutine to reset the num_failed_assert and num_assert values. Another option would be to make them PRIVATE and provide getter and setter functions but I considered that overkill for just two variables.

@certik
Copy link
Member

certik commented Jan 3, 2020

I think for testing, it's better to use or develop a more specific framework just for testing.

I think the main advantage of the assert macro is to test things like sizes of arrays when entering a subroutine, and other conditions that the code depends on. And in Release mode the macro is not executed, so there is no overhead.

@ivan-pi
Copy link
Member

ivan-pi commented Feb 2, 2020

Have we considered using fypp for assert macros? We already have it as a dependency.

@certik
Copy link
Member

certik commented Mar 23, 2020

To clarify: this issue is for Debug time assert statements, that get skipped in Release mode. Testing framework is a different use case, to be discussed in #162. Note however, that a project like stdlib will have a test suite, executed by the testing framework (#162), and this will call functions from stdlib such as save_txt() to ensure they work, and those functions internally would have ASSERT macros that would ensure that things are internally consistent (in Debug mode).

@milancurcic, @everythingfunctional I think stdlib should have such a macro.

However, the issue is that then by default the files ending with .f90 are not pre-processed by default, so the macro would not work. I can see two approaches:

  1. rename all files to .F90
  2. pass the proper compiler flag to the Fortran compiler in CMake (later on, once we switch to fpm, it would do the same)

A third approach is to only do this for files that would use ASSERT. I don't like this third option, as we should be free to use ASSERT in any file, without having to fiddle with the build system or with a file extension.

Which approach do you prefer, 1) or 2)?

I think I prefer 2), simply because using upper case extension feels old fashioned, I am not aware of any modern language that would require that. See also j3-fortran/fortran_proposals#56.

@everythingfunctional
Copy link
Member

I think I would lean towards 2. A standard preprocessor is something Fortran probably should have settled on a long time ago. There are simply too many use cases for macros and preprocessor directives not too. I would be fine if fpm picked a preprocessor and just always used it, whether that file needed it or not.

@certik
Copy link
Member

certik commented Mar 23, 2020

@everythingfunctional exactly. See also j3-fortran/fortran_proposals#65 for a related discussion about standardizing a pre-processor.

The only issue that I can see is that the typical CPP preprocessor is pretty limited in what it can do, and it does not understand Fortran's syntax. fypp is an improvement. Even better would be to design intelligent macros for Fortran. That is all long term, a short term solution is to use CPP. However, I wouldn't like our choice to prevent us to implement intelligent macros in the future. However, for this we could use the edition keyword (j3-fortran/fortran_proposals#83) that we can specify in fpm.toml, just like Rust does it. So from some future moment, we could switch the pre-processor to something more intelligent and for legacy codes we would provide an option to explicitly keep using CPP for a particular file (one would have to specify that explicitly in fpm.toml if a newer edition was required).

@certik
Copy link
Member

certik commented Apr 2, 2020

Also note that the NAG compilers do not pre-process .F90 automatically, instead one has to use the -fpp option, per open-mpi/ompi#7584. So that leaves us with option 2, as option 1 would not work anyway in all compilers.

@nncarlson
Copy link
Member

NAG does automatically preprocess .F90 files. Mostly. The issue is on macOS with case insensitive file names as I understand it. But surely that must be a problem for all compilers that decide solely on the basis of the file name?

@aradi
Copy link
Member

aradi commented Apr 3, 2020

@certik Just to understand: According to your proposal some stdlibs files would then contain two different kind of macros: fypp-macros (when needed for generating "generic" interfaces) and cpp-style macros for the asserts? While technically possible (fypp's preprocessor instructions are orthogonal to the cpp instructions), wouldn't it be more to use only one preprocessor? Either cpp or fypp but not both? (And then, in case fypp is choosen, package it with stdlib (#133) to make sure, it does not become an extra dependency)?

@certik
Copy link
Member

certik commented Apr 3, 2020

I am not sure. It seems we agree to just stick to .f90 and ensure we call the Fortran compilers with proper options. This also gives us the option to just use fypp, and not cpp.

I think the best way forward is to simply try a few approaches and see which one works the best.

Can fypp handle creating ASSERT like macros?

@aradi
Copy link
Member

aradi commented Apr 3, 2020

Yes, of course, it is possible to write macros which depending on a flag/variable create different codes, see the according exampe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: utilities containers, strings, files, OS/environment integration, unit testing, assertions, logging, ...
Projects
None yet
Development

No branches or pull requests

8 participants