[PATCH] Some code improvements (no functional change)

[Duncan P. N. Exon Smith]

On 2014-Apr-30, at 4:12, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:

>>> 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.

Note that the change to include/llvm/Support/Unicode.h still LGTM.  Commit
that any time.

It seems wrong to me that you're including regex2.h, as it looks like private
implementation details for the BSD regex code.  Adding include guards makes
the headers look less like shared (but private) implementation details and
more like public interfaces -- that's misleading to anyone reading the code.
I still think these should be left as is, as the current setup discourages
anyone from including them.

However, I'm kind of new around here (maybe it's *my* ideas that are wrong),
and it's not like you're proposing something outrageous.

Anyone else care to chime in?