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

faster to_lower and to_upper #733

Merged
merged 8 commits into from Oct 13, 2023
Merged

faster to_lower and to_upper #733

merged 8 commits into from Oct 13, 2023

Conversation

jalvesz
Copy link
Contributor

@jalvesz jalvesz commented Aug 28, 2023

Following discussion here #703


do i = 1, len(string)
lower_string(i:i) = char_to_lower(string(i:i))
icar= ichar(string(i:i))
Copy link
Member

Choose a reason for hiding this comment

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

Why did you not modify char_to_lower instead of each occurence?

@@ -223,31 +223,27 @@ contains
pure function char_to_lower(c) result(t)
character(len=1), intent(in) :: c !! A character.
character(len=1) :: t
integer, parameter :: wp= 32, la=65, lz= 90
Copy link
Member

Choose a reason for hiding this comment

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

What are the origin of these numbers?

Choose a reason for hiding this comment

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

I would prefer something like

integer, parameter :: la=iachar('A'), lz=iachar('Z')

as 65 is the ADE (ASCII Decimal Equivalent) of capital "A", and 90 is the ADE for capital Z". The advantage of numbers is that they are case-insensitive; but in this case I think the letters are much clearer. Longer descriptive names for the variables would be nice as well IMHO.

DECIMAL
*-------*-------*-------*-------*-------*-------*-------*-------*
| 00 nul| 01 soh| 02 stx| 03 etx| 04 eot| 05 enq| 06 ack| 07 bel|
| 08 bs | 09 ht | 10 nl | 11 vt | 12 np | 13 cr | 14 so | 15 si |
| 16 dle| 17 dc1| 18 dc2| 19 dc3| 20 dc4| 21 nak| 22 syn| 23 etb|
| 24 can| 25 em | 26 sub| 27 esc| 28 fs | 29 gs | 30 rs | 31 us |
| 32 sp | 33  ! | 34  " | 35  # | 36  $ | 37  % | 38  & | 39  ' |
| 40  ( | 41  ) | 42  * | 43  + | 44  , | 45  - | 46  . | 47  / |
| 48  0 | 49  1 | 50  2 | 51  3 | 52  4 | 53  5 | 54  6 | 55  7 |
| 56  8 | 57  9 | 58  : | 59  ; | 60  < | 61  = | 62  > | 63  ? |
| 64  @ | 65  A | 66  B | 67  C | 68  D | 69  E | 70  F | 71  G |
| 72  H | 73  I | 74  J | 75  K | 76  L | 77  M | 78  N | 79  O |
| 80  P | 81  Q | 82  R | 83  S | 84  T | 85  U | 86  V | 87  W |
| 88  X | 89  Y | 90  Z | 91  [ | 92  \ | 93  ] | 94  ^ | 95  _ |
| 96  ` | 97  a | 98  b | 99  c |100  d |101  e |102  f |103  g |
|104  h |105  i |106  j |107  k |108  l |109  m |110  n |111  o |
|112  p |113  q |114  r |115  s |116  t |117  u |118  v |119  w |
|120  x |121  y |122  z |123  { |124  | |125  } |126  ~ |127 del|
*-------*-------*-------*-------*-------*-------*-------*-------*

@@ -223,11 +223,11 @@ contains
pure function char_to_lower(c) result(t)
character(len=1), intent(in) :: c !! A character.
character(len=1) :: t
integer, parameter :: wp= 32, la=65, lz= 90
integer, parameter :: wp= 32, BA=iachar('A'), BZ=iachar('Z')
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could do the same for wp:

Suggested change
integer, parameter :: wp= 32, BA=iachar('A'), BZ=iachar('Z')
integer, parameter :: wp= iachar('A')-iachar('a'), BA=iachar('A'), BZ=iachar('Z')

Or is it not correct?

Copy link
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @jalvesz for this improvement.

@jvdp1
Copy link
Member

jvdp1 commented Oct 13, 2023

I'll merge this PR. Thank you @jalvesz for these changes.

@jvdp1 jvdp1 merged commit 4204079 into fortran-lang:master Oct 13, 2023
17 checks passed
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.

None yet

3 participants