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

Minor update pure/elemental in string_type module #562

Merged
merged 2 commits into from
Nov 13, 2021

Conversation

zoziha
Copy link
Contributor

@zoziha zoziha commented Nov 12, 2021

  • pure elemental -> elemental: ✔
  • interface string_type procedure
    • new_string
    • new_string_from_integer
    • new_string_from_logical
  • pure -> elemental: ❓(Update: ❌)
  • interface char procedure
    • char_string
    • char_string_range
  • private function maybe
  • add elemental: ❓(Update: ✔)
  • interface move procedure
    • move_string_string
  • add pure: ✔
  • interface move procedure
    • move_char_string
    • move_char_string
    • move_char_char

Description

Here I have updated the elemental attributes of the routine. There are two areas that are doubtful ❓, but I think they can be elemental.

Another proposal

There is another issue.
string_type interface relies on the to_string routine, it can convert character, integer, and logical variables into strings and be elemental because type(string_type) can be non-allocatable, but it does not support the format argument.
While to_string supports integer, real, complex and logical types, support format argument, but not elemental.

string_type seems to have a elemental advantage to construct strings. It seems that we can take advantage of this and let string_type add support for real and complex types, and support the format argument?

@milancurcic
Copy link
Member

 pure -> elemental: 
interface char procedure
char_string
char_string_range
private function maybe
 add elemental: 
interface move procedure
move_string_string

I also think both of these are safe to be elemental. In the first case we go from character to string_type, so an input character array will need to be fixed length no matter what. In the second case it's all string_type to string_type so I don't see an issue.

Others are cosmetic and straightforward, so I think this PR can go forward.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

There are some limitations in char_string and char_string_range which lead me to use pure rather than elemental. Namely, there is an issue with elemental which can result in an uncatchable runtime error. While useful, the subtle consequences of elemental usage might lead to hard to debug runtime errors for some use cases.

Add test for `elemental` `move`.
@zoziha
Copy link
Contributor Author

zoziha commented Nov 13, 2021

Thanks @awvwgk , I understand what you mean now.
char_string, char_string_range, maybe, I kept their pure attributes, and add test code for elemental move of type(string_type).

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me.

@milancurcic milancurcic merged commit 6033397 into fortran-lang:master Nov 13, 2021
@zoziha zoziha deleted the to-elemental branch November 14, 2021 01:13
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.

3 participants