On 11/23/18 8:06 AM, Martin Liška wrote:
Hi.

It's patch proposal that suggests to use an enum instead of 'int endp' for
functions that expand memory move builtins. I've touch the code multiple times
and it always take me time to realize what the magic numbers 0, 1, 2 mean.

Does the suggestion make sense? Do you like enum values? Can it be installed 
now?
If so I can finish the patch (few targets must be ported and ChangeLog entry is 
missing).

I think an enum will make the code much more readable.

I'm not sure the name choices are the most intuitive.  Maybe
something like this instead?

  RETURN_BEGIN, RETURN_END, RETURN_END_MINUS_1

(I suggest BEGIN rather than START because of C++ iterators, but
I see the bigger readability gain in using RETURN over POINTER;
IMO, it would be also nice to rename endp to retp or retmode
or something like that).

For the RETURN_END_MINUS_1 comment, I would just should say
"return a pointer to the terminating null byte of the string,"
rather than "to the end of it."  (The latter might seem to
contradict RETURN_END).

Martin

Reply via email to