[PATCH] Some code improvements (no functional change)
Artyom.Skrobov at arm.com
Tue May 13 10:11:35 PDT 2014
>>> Yes, this looks better. Send the full patch and I'll have another look.
>> Thank you! Attaching the updated patch.
Committed as r208680 and r208684
> Note that the change to include/llvm/Support/Unicode.h still LGTM. Commit
> that any time.
Committed as r208681
> It seems wrong to me that you're including regex2.h, as it looks like
> implementation details for the BSD regex code. Adding include guards
> the headers look less like shared (but private) implementation details and
> more like public interfaces -- that's misleading to anyone reading the
> 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/Target/PowerPC/PPCPerfectShuffle.h and the 4 regex headers in question)
-- I think this shows a prominent tendency to have such guards in private
> However, I'm kind of new around here (maybe it's *my* ideas that are
> and it's not like you're proposing something outrageous.
> Anyone else care to chime in?
More information about the llvm-commits