-
Notifications
You must be signed in to change notification settings - Fork 184
promote other ascii functions to elemental #977
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this improvement @wassup05.
I have restored the former table function.
A good test case would be to reproduce the results of the table as reported at https://en.cppreference.com/w/cpp/string/byte
the comparison table should look like
logical, parameter :: ascii_class_table(15,12) = transpose(reshape([ &
! iscntrl isprint isspace isblank isgraph ispunct isalnum isalpha isupper islower isdigit isxdigit
.true., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false., & ! 0–8
.true., .false., .true., .true., .false., .false., .false., .false., .false., .false., .false., .false., & ! 9
.true., .false., .true., .false., .false., .false., .false., .false., .false., .false., .false., .false., & ! 10–13
.true., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false., & ! 14–31
.false., .true., .true., .true., .false., .false., .false., .false., .false., .false., .false., .false., & ! 32 (space)
.false., .true., .false., .false., .true., .true., .false., .false., .false., .false., .false., .false., & ! 33–47
.false., .true., .false., .false., .true., .false., .true., .false., .false., .false., .true., .true., & ! 48–57
.false., .true., .false., .false., .true., .true., .false., .false., .false., .false., .false., .false., & ! 58–64
.false., .true., .false., .false., .true., .false., .true., .true., .true., .false., .false., .true., & ! 65–70
.false., .true., .false., .false., .true., .false., .true., .true., .true., .false., .false., .false., & ! 71–90
.false., .true., .false., .false., .true., .true., .false., .false., .false., .false., .false., .false., & ! 91–96
.false., .true., .false., .false., .true., .false., .true., .true., .false., .true., .false., .true., & ! 97–102
.false., .true., .false., .false., .true., .false., .true., .true., .false., .true., .false., .false., & ! 103–122
.false., .true., .false., .false., .true., .true., .false., .false., .false., .false., .false., .false., & ! 123–126
.true., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false., .false. & ! 127
], shape=[12,15]))
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @wassup05. I think this PR is ready to merge imho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you @wassup05
Could you also update the specs where/if it is required, please?
Another way of performing the test and validating the 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)? |
Yes, please add them to https://github.com/fortran-lang/stdlib/blob/master/doc/specs/stdlib_ascii.md , thank you. |
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 togfortran
) and as thatsubroutine
was not used anywhere in the tests, is now removed.Maybe we should add some array test cases to ensure it behaves correctly?