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

Fix usage of invalid BOZ integer constants #44

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

albertziegenhagel
Copy link

@albertziegenhagel albertziegenhagel commented Aug 29, 2020

gfortran >= 10.0 started to emit an error for BOZ integer constants in invalid locations. This change replaces the hexadecimal integer constants by regular ones, so that msmpi fortran bindings can be consumed by gfortran without the need to pass -fallow-invalid-boz.

Some amount of integer constants have already been replaced in #25.

edit: The conversions from the hexadecimal numbers to decimal numbers have been performed by a simple python script, so I hope there should not be any typos.

gfortran >= 10.0 started to emit an error for BOZ integer constants in invalid locations. This change replaces the hexadecimal integer constants by regular ones, so that msmpi fortran bindings can be consumed by gfortran without the need to pass `-fallow-invalid-boz`.
@KineticTheory
Copy link

👍 Thanks for putting this fix together. I'm seeing the same issues with gfortran.

@scivision
Copy link

Thank you. Currently there are a large volume of warnings emitted before this fix. Any way we can help expedite?

@KineticTheory
Copy link

@scivision @albertziegenhagel Do you know of a gfortran compiler flag or code pragma that will suppress this warning? I would like to silence this warning in my Appveyor CI. Thanks!

KineticTheory added a commit to KineticTheory/Microsoft-MPI that referenced this pull request Apr 17, 2021
When using MS-MPI's `mpif.h` with gfortran version 10+ there are many warnings of the form:
` warning : BOZ literal constant at (1) is neither a data-stmt-constant nor an actual argument to INT, REAL, DBLE, or CMPLX intrinsic function`.  The eliminate these warnings convert the hex data types to integers.  This extends the work started in microsoft#25.

fixes microsoft#44
Copy link

@zoziha zoziha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that this PR has existed for 4 years. Recently, I have also come across such warnings and discovered this repo. These warnings arise from the code writing not conforming to the Fortran standard, and they are frequently historical extensions of the Fortran compilers.

@albertziegenhagel, thank you for sharing!

PARAMETER (MPI_DATATYPE_NULL=z'0c000000')
PARAMETER (MPI_DATATYPE_NULL=201326592)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another modification approach is PARAMETER (MPI_DATATYPE_NULL=int(z'0c000000')), because int is a built-in function, and compilation optimization makes it efficient. This modification might be easily reviewed.

Of course, directly updating to decimal is also good.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is convenient, subsequent commits can also update the use of similar non-standard syntax in mpi.f90:

PARAMETER (MPI_DATATYPE_NULL=z'0c000000')

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gfortran may throw out the following warning(s):

mpif.h:56:19:
Warning: Obsolescent feature: Old-style character length at (1)
mpif.h:57:19:
Warning: Obsolescent feature: Old-style character length at (1)

The non-standard writing character*1 should be replaced with the standard writing character(1):

CHARACTER*1 MPI_ARGVS_NULL(1,1)

CHARACTER*1 MPI_ARGVS_NULL(1,1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants