[PATCH] Some code improvements (no functional change)

Artyom Skrobov Artyom.Skrobov at arm.com
Thu May 22 09:06:47 PDT 2014


Hello Duncan,

Besides pinging on the discussion on include guards in private headers,
quoted below, I'd like to ask your opinion on naming the byte-swapping
functions swapByteOrder for the in-place variants, and getBytesSwapped for
the by-value/return variants; what do you think?
In addition to adhering to the lowercase-initial convention advised in the
Coding Standards, I believe this would also reflect more clearly what the
functions are doing, than their current names of SwapValue and
SwapByteOrder.


-----Original Message-----
From: Artyom Skrobov [mailto:Artyom.Skrobov at arm.com] 
Sent: 13 May 2014 18:12
To: 'Duncan P. N. Exon Smith'
Cc: llvm-commits
Subject: RE: [PATCH] Some code improvements (no functional change)

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

Curiously enough, lib/Support/regex_impl.h is (and has always been) guarded
by #ifndef _REGEX_H_ -- that's although the header's own name indicates it's
a part of the implementation, and not an interface (public nor private).
I've tried, and removing that guard doesn't have any effect on LLVM
compilation; I can conclude that the historic (non-)usage of include guards
in the regex headers isn't consistent enough to be taken as a guidance. 

Out of the 395 headers in lib/, supposedly none of them belonging to LLVM
public interface, 384 do have re-include guards, and only 11 don't (that's
lib/Support/Windows/WindowsSupport.h lib/Target/ARM/ARMPerfectShuffle.h
lib/Target/ARM64/ARM64PerfectShuffle.h
lib/Target/Hexagon/HexagonVarargsCallingConvention.h
lib/Target/NVPTX/NVPTXISelDAGToDAG.h
lib/Target/NVPTX/NVPTXMachineFunctionInfo.h
lib/Target/PowerPC/PPCPerfectShuffle.h and the 4 regex headers in question)
-- I think this shows a prominent tendency to have such guards in private
headers.

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








More information about the llvm-commits mailing list