Skip to content

Wrt stdlib_ascii.f90: use iachar/achar consistently #232

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

Open
arjenmarkus opened this issue Sep 14, 2020 · 6 comments
Open

Wrt stdlib_ascii.f90: use iachar/achar consistently #232

arjenmarkus opened this issue Sep 14, 2020 · 6 comments
Labels
implementation Implementation in experimental and submission of a PR topic: strings String processing

Comments

@arjenmarkus
Copy link
Member

While having a quick look at the documentation, I stumbled upon the implementation of the ASCII functions. For instance:

pure logical function is_alpha(c) character(len=1), intent(in) :: c !! The character to test. is_alpha = (c >= 'A' .and. c <= 'Z') .or. (c >= 'a' .and. c <= 'z') end function

The implementation assumes that the character that is passed is in ASCII encoding. While ubiquitous nowadays, it is certainly not the only possible encoding. For stdlib we should be as general as possible, Here is a possible alternative for the is_alpha() function:

pure logical function is_alpha(cin)
    character(len=1), intent(in) :: cin !! The character to test.
    integer :: c
    c = iachar(cin)
    is_alpha = (c >= iachar('A') .and. c <= iachar('Z')) .or. (c >= iachar('a') .and. c <= iachar('z'))
end function
@arjenmarkus
Copy link
Member Author

Perhaps it is over cautious, but it cannot hurt to consider this.

@certik
Copy link
Member

certik commented Sep 14, 2020

Can you send this as a Pull Request (PR)? I think this change is fine.

@arjenmarkus
Copy link
Member Author

arjenmarkus commented Sep 15, 2020 via email

arjenmarkus added a commit that referenced this issue Sep 16, 2020
Consistently use iachar() to avoid potential problems with platforms that use a different encoding than ASCII - see issue #232
@arjenmarkus
Copy link
Member Author

arjenmarkus commented Sep 16, 2020 via email

arjenmarkus added a commit that referenced this issue Sep 16, 2020
Commit the corrected source (#232)
@14NGiestas
Copy link
Member

I think you should revert the commit so we can fully review the changes, if I'm not mistaken, there is already a missing line in your changes.

@arjenmarkus
Copy link
Member Author

arjenmarkus commented Sep 16, 2020 via email

@awvwgk awvwgk added topic: strings String processing implementation Implementation in experimental and submission of a PR labels Sep 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
implementation Implementation in experimental and submission of a PR topic: strings String processing
Projects
None yet
Development

No branches or pull requests

4 participants