[libcxx-commits] [PATCH] D98207: [SystemZ][z/OS] Missing wchar functions libc++

[Muiez Ahmed via Phabricator via libcxx-commits]

muiez added inline comments.


================
Comment at: libcxx/src/support/ibm/mbsnrtowcs.cpp:42
+
+  while (source_remaining && (!dst || dest_converted < max_dest_chars)) {
+    // Converts one multi byte character.
----------------
DanielMcIntosh-IBM wrote:
> By tweaking this condition a bit we can get rid of `source_remaining` as a state variable: `source_converted < src_size_bytes && (!dst || dest_converted < max_dest_chars)`
> 
> You may still want to keep `source_remaining` around, but declare it inside the loop so it's no longer used as a state variable (`size_t source_remaining = src_size_bytes - source_converted;`). 
> 
> This also allows you to make it obvious that `source_converted` and `dest_converted` are only declared outside the for loop so that we can read from them after: After moving all the declarations except `terminated_sequence`, `invalid_sequence`, `incomplete_sequence`, `source_converted`, and `dest_converted`, the only non-const variables you've declared before the loop are `source_converted`, and `dest_converted`. If you move the zero-initialization to the init statement of the for loop, this signals that all writes to these two variables happen within the for loop, and after we only read from them (even better, we'd actually be doing all the writing **within the init and increment of the loop statement**, showing the reader right away what the pre-conditions and post-conditions are for each loop iteration).
Yup, I tweaked the condition to remove the `source_remaining` state variable. 

On the other hand, I attempted to use a for loop to include the init, condition & increment; however, this turned out to be tedious (i.e lots of variables, conditions, increments, etc.). For example we'd atleast have the following:

```
for( size_t source_converted = 0;
    source_converted < src_size_bytes && (!dst || dest_converted < max_dest_chars);
    source_converted += result,
    ++dest_converted) {
      
    }
```


Nonetheless, since we return `dest_converted` and must declare it out of the loop, then it might be more concise to initialize it upon declaration too. Also, since writing (*src) is conditional, a while loop is more appropriate.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98207/new/

https://reviews.llvm.org/D98207