[PATCH] Some code improvements (no functional change)

[Artyom Skrobov]

>> It appears that Duncan no longer has the availability to review my three
>> suggested patches -- perhaps someone else could take a look?
>
> Is this urgent?  If not, typical ping rate is 1 x week [1].

No, this is not urgent, and in fact I have been re-posting my review
requests once a week, exactly as specified in the developer policy.

> Yes, this looks better.  Send the full patch and I'll have another look.

Thank you! Attaching the updated patch.

>> To be fair, regutils.h had been modified once since then (r81114); but is
>> there any reason not to add the guards?
>
> There are cases were header guards are incorrect (break functionality),
> but I doubt this is one of them.  Nevertheless, I don't see any reason for
> code churn for the sake of churn.

The reason these headers attracted our attention is that in our compilation
environment, regex2.h gets double-included, and we thought if we add the
guards to one of these headers, we should use the opportunity to add them to
all four that were missing the guards.
I appreciate that this makes very little difference in trunk LLVM one way or
the other, and we certainly can keep this patch downstream if it's
unadvisable to be included into trunk LLVM, but I still don't see any costs
to this change, or any possible negative effects it could cause in trunk
LLVM.

>> Would it be reasonable if we name both SwapByteOrder() -- it's difficult
to
>> describe their purpose in any other way -- and make the in-place function
>> take a pointer, instead of a reference?
>
> Pointer is the wrong API: it implies having to check for null.

I see that in general, the choice between pointer parameters and reference
parameters can mean one of many things: input vs output, change of ownership
vs no change, validity of NULL, and perhaps more.

For example, Google C++ Style Guide mandates: "All parameters passed by
reference must be labeled const. [...] it is a very strong convention in
Google code that input arguments are values or const references while output
arguments are pointers." (
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Reference_Arg
uments )

I see that LLVM Coding Standards document doesn't touch this subject at all.
Should we use this opportunity to add to it that in LLVM, the choice between
pointer parameters and reference parameters is defined by whether NULL is a
valid input?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lib-Target.patch
Type: application/octet-stream
Size: 2740 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140430/385f29b4/attachment.obj>