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 low level version of read_formatted #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

certik
Copy link

@certik certik commented Feb 10, 2021

And use it in the high level version of read_formatted.

And use it in the high level version of read_formatted.
@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

The main point of read_formatted is that it is exported via

interface read(formatted)
  module procedure :: read_formatted
end interface

as user defined derived type input, this is not possible for intrinsic data types like character(len=:), allocatable because there is an intrinsic read transfer defined for character(len=*). Apparently, GCC let's you overload the read(formatted) interface even for an intrinsic data type, but if you try to use it with

program demo
  use stdlib_string_type, only : read(formatted)
  implicit none
  character(len=:), allocatable :: dlc
  integer :: io
  open(newunit=io, form="formatted", status="scratch")
  write(io, *) "Important saved value"
  write(io, *)

  rewind(io)

  read(io, *) dlc
  close(io)
end program demo

It will crash with a segmentation fault in the Fortran runtime, but not reach the read_formatted0 procedure at all:

 + build/gfortran_debug/test/test_string_derivedtype_io 

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0  0x7f105d21d69f in ???
#1  0x7f105d345b0f in ???
#2  0x7f105d7b347b in list_formatted_read_scalar
	at /build/gcc/src/gcc/libgfortran/io/list_read.c:2270
#3  0x558f28f11885 in __test_string_derivedtype_io_MOD_test_formatted_io
	at test/test_string_derivedtype_io.f90:24
#4  0x558f28f119b0 in tester
	at test/test_string_derivedtype_io.f90:55
#5  0x558f28f119ee in main
	at test/test_string_derivedtype_io.f90:52
 Command failed
ERROR STOP

As I said, user defined derived type input output is a bad example, because it cannot be defined for intrinsic data types.

@certik
Copy link
Author

certik commented Feb 10, 2021

Ok, I didn't realize you are overloading intrinsic read. Is this an issue in gfortran or the Fortran standard itself?

As a consequence, I don't think we should be overloading the intrinsic read, but rather provide a different name for this function.

@certik
Copy link
Author

certik commented Feb 10, 2021

Or to rephrase: if overloading works for string_type then maybe we can do it for that type. But string_type is a high level API as far as I am concerned. We can call it middle level also if there are plans for an even higher level API. But it is not a low level API. A low level API is the function read_formatted0 introduced in this PR. The function read_formatted is the high (or middle) level API.

@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

There is no issue with the Fortran standard, but GCC seems to miss a violation of the standard here (not 100% sure about this, I would have to look for the respective sections on UDDTIO in the specs).

@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

But string_type is a high level API as far as I am concerned.

Yes. The low level API it is wrapping right now are only intrinsic procedures. There is no need middle level because the low level API is in fact defined by the Fortran standard for character(len=*) which does not allow to define an additional middle API with character(len=:), alloctable.

@certik
Copy link
Author

certik commented Feb 10, 2021

Well, you actually provide some actual low level functionality. I went over your code and extracted all genuine functionality into a low level API, and then I use the low level API in your high level API.

So at this point, the file stdlib_string_type.f90 is a high level API which consists of wrapping either intrinsic or the low level API from stdlib_string_type_low_level.f90. There is no actual functionality, just wrappers.

@ivan-pi
Copy link

ivan-pi commented Feb 10, 2021

I don't really see the benefit of splitting those routines into a "low-level" API, if their only purpose is to be called in the exploratory high-level API.

The issue of a read command for character(len=:), allocatable, which would automatically allocate the a sequence of the right length should be rather addressed at the Standard level. An issue (opened by @certik) is already open for it at the j3-fortran GitHub page: j3-fortran/fortran_proposals#9

Edit: essentially I agree with the words of @awvwgk:

However, I disagree on the low level API for user defined derived type input output, it is strictly a feature that can only be defined for a derived type but not an intrinsic and we won't be able to make use of it to safely read into a character(len=:), allocatable :: dlc with read(unit, *) dlc due to the construction of the Fortran standard.

Towards the bottom of the thread in the j3-page proposals there is a link to comp.lang.fortran exchange: https://groups.google.com/g/comp.lang.fortran/c/9_0q0dBRzpk, the messages from 9. Jan and further contain a discussion of some of the ambiguities allocate on read would lead to.

@certik
Copy link
Author

certik commented Feb 10, 2021

Indeed, the read_formatted0 routine is a temporary solution until the Fortran Standard itself implements it.
It is useful to be able to do this without needing the string_type.

The other is maybe0 which actually is very useful: it allows you to use allocatable character and always convert it to a string even if it is unallocated (=> empty string).

Finally, the unused_dummy_argument should go into stdlib. @awvwgk do you want to propose it?

What exactly is then the advantage of the string_type?

It seems that it allows to override read, and you can use it as arrays of strings.

@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

What exactly is then the advantage of the string_type?

It is just a non-fancy string type, which basically provides the same functionality as a deferred length character but can be used in an elemental rather than a pure way. The idea was to have a scaffold for the string type in stdlib which can be extended later but already provides everything we are used to have from the deferred length character without the rough edges (not being able to read into an unallocated character, cannot have arrays).

@milancurcic
Copy link

Finally, the unused_dummy_argument should go into stdlib. @awvwgk do you want to propose it?

Please, no! :)

Let's not hack around compiler warnings with subroutines that don't do anything. Instead, we should disable the compiler warnings we don't like.

@certik
Copy link
Author

certik commented Feb 10, 2021

It is just a non-fancy string type, which basically provides the same functionality as a deferred length character but can be used in an elemental rather than a pure way. The idea was to have a scaffold for the string type in stdlib which can be extended later but already provides everything we are used to have from the deferred length character without the rough edges (not being able to read into an unallocated character, cannot have arrays).

Would you mind adding this paragraph to the file as a comment (at the top) or to the specs or both?

@certik
Copy link
Author

certik commented Feb 10, 2021

Finally, the unused_dummy_argument should go into stdlib. @awvwgk do you want to propose it?

Please, no! :)

Let's not hack around compiler warnings with subroutines that don't do anything. Instead, we should disable the compiler warnings we don't like.

Putting my compile writer hat on, how do you disable a warning just for a subroutine? Is this warning even useful at all?

@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

On @certik suggestion I went ahead to propose the function at fortran-lang/stdlib#316, I don't think it is a good addition to stdlib because it is a hack to quickly mute a warning in an incomplete implementation.

@milancurcic
Copy link

Putting my compile writer hat on, how do you disable a warning just for a subroutine? Is this warning even useful at all?

Hmm, good point, I don't know if that's possible, but only disabling it per source file. I think the warning is useful when it's a true positive, but often gfortran will throw spurious warnings (unused in this case, or uninitialized var in case of allocatable strings).

But all I can say is, "I don't like it", but it's possible that it could be useful. So if others support it, I will too.

@milancurcic
Copy link

This could be a good excuse to have a stdlib_hacks module.

@awvwgk
Copy link
Owner

awvwgk commented Feb 10, 2021

The other is maybe0 which actually is very useful: it allows you to use allocatable character and always convert it to a string even if it is unallocated (=> empty string).

Agreed, opened an issue for further discussion at fortran-lang/stdlib#317.

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