<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">On Nov 10, 2013, at 5:42 AM, Alp Toker <<a href="mailto:alp@nuanti.com">alp@nuanti.com</a>> wrote:<div><blockquote type="cite">
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
<div bgcolor="#FFFFFF" text="#000000">
With the recent thread on using C++11 in LLVM/clang, one of the
recurring themes was a desire to make the internal headers more
consumable.<br></div></blockquote><div><br></div><div>Great! I strongly agree with your goals, though have a few questions below:</div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><b>#ifndef NDEBUG</b><br>
<br>
This is the biggest violation. NDEBUG should only ever be used in
source files. That way if something is crashing we can swap in a
debug build without rebuilding every single dependent application.
Win!<br></div></blockquote><div><br></div><div>+1</div><div><br></div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><b>undefs</b><br>
<br>
Some examples from llvm/include and clang/include:<br>
<br></div></blockquote><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><code>#undef INT64_MAX</code><code><br></code></div></blockquote><div><br></div><div><div>INT64_MAX is specific to AIX. I'm not sure that we even build on AIX anymore, but in any case, the people who care about AIX can figure out what they want to do.</div><div><br></div><div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><code>#undef alloca</code><br></div></blockquote></div></div><div><br></div><div>This looks like a problem in clang/include/Basic/Builtins.h. I agree it should be fixed.</div><div><br></div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<code>#undef NetBSD</code><code><br>
</code><code>#undef mips</code><code><br>
</code><code>#undef sparc</code><br></div></blockquote><div><br></div>These (and the other ones in llvm/Support/Solaris.h) are fine and we should keep them. These exist because of system headers that trample on the global namespace. Each of these is also defined with an __ version as well, and are only preserved for legacy reasons.</div><div><br></div><div>The only impact of LLVM containing these is that code cannot use the legacy names *and* include an LLVM header. It seems perfectly reasonable for me to require code to use the __ version of a name if it is updating to use LLVM headers. Adding new features to legacy code makes that code non-legacy in my books.</div><div><br></div><div>-Chris</div><div><br></div><div><br></div></body></html>