fortran-lang
promote other ascii functions to elemental
Attempts to address #973
There was a subroutine which would not compile due to this change as procedure pointers cannot point to elemental procedures (according to gfortran) and as that subroutine was not used anywhere in the tests, is now removed.
Maybe we should add some array test cases to ensure it behaves correctly?
Thank you very much for the refactor @perazz
I have added the test incorporating the procedure, do let me know what you think about it.
Another way of performing the test and validating the elemental attribute to check simultaneously all elements could be:
subroutine test_ascii_table
integer :: i, j
logical :: table(15,12)
type :: tests
character(1), allocatable :: chars(:)
end type
type(tests) :: my_tests(15)
my_tests(1)%chars = [(achar(j),j=0,8)] ! control codes
my_tests(2)%chars = [(achar(j),j=9,9)] ! tab
my_tests(3)%chars = [(achar(j),j=10,13)] ! whitespaces
my_tests(4)%chars = [(achar(j),j=14,31)] ! control codes
my_tests(5)%chars = [(achar(j),j=32,32)] ! space
my_tests(6)%chars = [(achar(j),j=33,47)] ! !"#$%&'()*+,-./
my_tests(7)%chars = [(achar(j),j=48,57)] ! 0123456789
my_tests(8)%chars = [(achar(j),j=58,64)] ! :;<=>?@
my_tests(9)%chars = [(achar(j),j=65,70)] ! ABCDEF
my_tests(10)%chars = [(achar(j),j=71,90)] ! GHIJKLMNOPQRSTUVWXYZ
my_tests(11)%chars = [(achar(j),j=91,96)] ! [\]^_`
my_tests(12)%chars = [(achar(j),j=97,102)] ! abcdef
my_tests(13)%chars = [(achar(j),j=103,122)]! ghijklmnopqrstuvwxyz
my_tests(14)%chars = [(achar(j),j=123,126)]! {|}~
my_tests(15)%chars = [(achar(j),j=127,127)]! backspace character
! loop through functions
do i = 1, 15
table(i,1) = all(is_control(my_tests(i)%chars))
table(i,2) = all(is_printable(my_tests(i)%chars))
table(i,3) = all(is_white(my_tests(i)%chars))
table(i,4) = all(is_blank(my_tests(i)%chars))
table(i,5) = all(is_graphical(my_tests(i)%chars))
table(i,6) = all(is_punctuation(my_tests(i)%chars))
table(i,7) = all(is_alphanum(my_tests(i)%chars))
table(i,8) = all(is_alpha(my_tests(i)%chars))
table(i,9) = all(is_upper(my_tests(i)%chars))
table(i,10) = all(is_lower(my_tests(i)%chars))
table(i,11) = all(is_digit(my_tests(i)%chars))
table(i,12) = all(is_hex_digit(my_tests(i)%chars))
end do
! output table for verification
write(*,'(5X,12(I4))') (i,i=1,12)
do j = 1, 15
write(*,'(I3,2X,12(L4),2X,I3)') j, (table(j,i),i=1,12), count(table(j,:))
end do
write(*,'(5X,12(I4))') (count(table(:,i)),i=1,12)
end subroutine test_ascii_table
No need for procedure pointers either.
Thank you for your reviews everyone.
It seems these procedures are not documented at all. So there is nothing to change really. But maybe I should add them now? since they are complete with functionality now.
@jalvesz ohh yes that is indeed a way of doing two things at once and seems a good way to me. Maybe some other reviewers could also pitch in and let me know what they think about this way (since I wasn't the one who added the table procedure)?
But maybe I should add them now
Yes, please add them to https://github.com/fortran-lang/stdlib/blob/master/doc/specs/stdlib_ascii.md , thank you.
@wassup05 I was testing a methodology for collaboration and pushed directly to your PR. Hope it is fine ( ? ) in the last commit you'll find a mix of the test that @perazz proposed before and the propal from the comment. Feel free to change it.
I don't mind at all @jalvesz, thank you very much for the refactor.
ascii module was missing docs of the constants and some procedures (that are now elemental) it provides, I added the constants to the docs and will be adding the procedures shortly.
@wassup05 your last commit for the specs look good to me. Do you consider it done? If yes, I would say lets wait a couple of days in case there are some comments and then merge.
there is no need to store integer ascii codes in a derived type imho, it’s not immediately clear those are ascii codes.
The intention is not storing ascii codes in a DT for the sake of it, simply having a list of allocatable characters such that each one is seen as an array. Since each list has a different size, it can not be done directly with a 2D array of characters, which would have otherwise been the simplest case. The DT is there to build a Jagged Array. Thus exactly the same test is performed. The only difference being that the elemental attribute is being put to test as it is applied on a whole array at once for each line of the jagged array.
I would prefere to keep the current test which imo is shorter and easier to read.
Hello @wassup05 @jalvesz, when you are ready to revert commit https://github.com/fortran-lang/stdlib/commit/4bc022d9244cbf5438b602b69113be5097341461, then I believe this PR can be merged.
@perazz for the moment I haven't had the time to think about a different way of doing the test. As we discussed, the issue I had with the previous proposal is that the test did not enable doing the following:
... = all( is_<fun_name>( array(:) ) )
Which the current test does.
This is important because it is the only actual valuable difference between an elemental and a pure procedure (element-wise whole array evaluations). So, say that a compiler for some reason has a bug in the treatment of the elemental attribute, the current test would fail. The previous test would apply the same logic as if the procedure had no elemental attribute on it.
@perazz @jalvesz I think there are pros and cons to both the approaches, The elemental checking one proposed by @jalvesz does what it's for but I don't think we should be checking for compiler bugs on the application/library side, that's not exactly our issue, But it works I feel. Either way I think it is a small thing, since it's just a test after all. I think we should merge this regardless.