<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hi Andrew, I'm going to comment a bit out-of-order.</div><div><br></div><div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite"><div><span class="Apple-style-span" style="color: rgb(51, 51, 51); font-size: 13px; -webkit-border-horizontal-spacing: 2px; -webkit-border-vertical-spacing: 2px; ">-the feature was not integrated into our PCH validation, to be more specific, validating and giving errors when assertions from PCH clash with predefined assertions or assertions from the command-line</span></div></blockquote>I do not believe there is anything that needs to be validated in terms of PCH vs command-line vs source preprocessor assertions. You can assert multiple values for a given key quite correctly (unlike macros) and assertions are designed to be added and removed using all of these features and it is not wrong to do so. Further, asserting the same key-value pair twice is not an error and so I don't think there needs to be validation of clashes since, by design, these clashes are permitted.<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite"></blockquote></div></blockquote><br></div><div>I had in mind checking whether there are differences in the assertions in command-line and predefined ones, (missing assertions, newly introduced), causing differences in PCH inclusion, like we do with macros, but I noticed that gcc does not care for assertion differences in command-line (when including the PCH) and even if the predefined ones are different, e.g including a PCH with</div><div><br></div><div>#assert cpu(i386)</div><div><br></div><div>when you are on x86_64, the PCH will probably be rejected by PCH validation anyway, so we probably don't need to enhance validation to include assertions.</div><div><br></div><div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"><blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite"><div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px; -webkit-border-horizontal-spacing: 2px; -webkit-border-vertical-spacing: 2px; ">So, to recap, the question is this, should we implement and maintain a gcc extension that was deprecated since gcc 3 ?</span></font></div></blockquote>I think clang should implement and maintain this feature because<br>1) It's widely supported by existing compilers including GCC 4.6.X (<a class="moz-txt-link-freetext" href="http://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Preprocessor-Options.html#Preprocessor-Options">http://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Preprocessor-Options.html#Preprocessor-Options</a>), Intel C/C++ (<a class="moz-txt-link-freetext" href="http://denali.princeton.edu/intel_cc_docs/c_ug/index.htm">http://denali.princeton.edu/intel_cc_docs/c_ug/index.htm</a>), and ARM's toolchain (<a class="moz-txt-link-freetext" href="http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CHDFDIDE.html">http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CHDFDIDE.html</a>)<br>2) Preprocessor assertions are an optional extension defined in System V Release 4 (SVR4) see the System V Interface manual Vol 3 page 270 (<a class="moz-txt-link-freetext" href="http://www.sco.com/developers/devspecs/vol3_ps.ps">http://www.sco.com/developers/devspecs/vol3_ps.ps</a>) and so should be implemented to support SVR4 systems which have chosen to implement this feature.<br>3) It was clearly identified as a missing feature by FIXMEs in Chris' original commit of PPDirectives.cpp and currently the compiler produces only cryptic errors when this feature is encountered.<br>4) It is not possible to work around this feature without having to modify the source and doing so with an automated sed or awk script is risky and difficult to integrate into some build processes. I did consider trying to emulate assertions using macros, but I found this wasn't possible using only the preprocessor. Asserts and macros exist in different namespaces and so you can define and assert the same symbol. In many cases this is not a problem because all we want to do is test if a macro is defined, but if the value of the macro is important then a collision in the names would cause a problem. The assertion tests themselves would have to be found and rewritten using a sed or awk script which is a fragile hack rather than a proper fix and having to run this script before a compile would complicate the build process etc. This kind of workaround also assumes that the source can be modified prior to compilation.<br>5) Clang has publicly stated that it will aim for GCC compatibility where possible and these patches make compatibility with this feature possible.<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite"></blockquote></div></blockquote><br></div><div>"aim for GCC compatibility" does not mean "implement the exact behavior of gcc unconditionally".</div><div>There are gcc extensions that we don't have interest in implementing, like nested functions, and when the standard disagrees with gcc we side with the standard, see <a href="http://clang.llvm.org/compatibility.html">http://clang.llvm.org/compatibility.html</a>.</div><div><br></div><div>It does not help the case for this feature that gcc itself is aggressively discouraging people from using it, e.g. this is from gcc 4.6.2:</div><div><br></div><div><div>$ cat t.c</div><div>#assert a(b)</div><div>#if #a(b)</div><div>#endif</div></div><div><br></div><div><div>$ gcc -fsyntax-only t.c</div><div>t.c:1:2: warning: #assert is a deprecated GCC extension [-Wdeprecated]</div><div>t.c:2:5: warning: assertions are a deprecated extension [-Wdeprecated]</div></div><div><br></div><div>I find the prospect that we would implement a new feature and then put it under a "deprecated" warning, at least unfortunate.</div><div><br></div><div>Since we in clang would discourage people from using the feature as well, we would implement it mainly to support legacy codebases or projects that, for some reason, still depend on a deprecated (and not widely used) gcc extension.</div><div>Under the this frame of discussion:</div><div><br></div><div><blockquote type="cite">I ran into preprocessor assertions when I was trying to build an older version of Grub which is what prompted my implementation.</blockquote><br></div><div>Could you elaborate more, is this legacy codebase that nobody wants to touch or move to a newer version ?</div><div>-Is the feature widely used in your company's codebase or not ?</div><div>-Is it only a minority of projects that you care about that depend on it ?</div><div>-Could these just keep using gcc for building ?</div><div><br></div><div>-Argyrios</div><div><br></div><br><div><div>On Mar 6, 2012, at 9:27 PM, Andrew Craik wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
Hi Argyrios,<br>
<br>
Thank you for reviewing my patches.<br>
<br>
I have replied to your comments inline below.<br>
<br>
I ran into preprocessor assertions when I was trying to build an
older version of Grub which is what prompted my implementation.<br>
<br>
Kind regards,<br>
Andrew<br>
<br>
On 07-Mar-12 03:21, Argyrios Kyrtzidis wrote:
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div>Hi all,</div>
<div><br>
</div>
<div>I'd like to get feedback and consensus on whether we should
add "GCC Preprocessor Assertions" to clang. This gcc extension
has been deprecated since gcc-3, here's a quote from gcc 3.1
docs, (bold added by me):</div>
<div><br>
</div>
<div>From <a moz-do-not-send="true" href="http://gcc.gnu.org/onlinedocs/gcc-3.1/cpp/Assertions.html">http://gcc.gnu.org/onlinedocs/gcc-3.1/cpp/Assertions.html</a></div>
<div><br>
</div>
<div>
<blockquote type="cite">Assertions are a <b>deprecated</b>
alternative to macros in writing conditionals to test what
sort of computer or system the compiled program will run on.
Assertions are usually predefined, but you can define them
with preprocessing directives or command-line options.<br>
<br>
Assertions were intended to provide a more systematic way to
describe the compiler's target system. However, in practice
they are just as unpredictable as the system-specific
predefined macros. In addition, they are not part of any
standard, and only a few compilers support them. Therefore,
the use of assertions is less portable than the use of
system-specific predefined macros. <b>We recommend you do not
use them at all</b>.</blockquote>
</div>
<div><br>
</div>
<div>Do note that, after checking on <a moz-do-not-send="true" href="http://ideone.com/">http://ideone.com</a>, gcc still
supports them on <span class="Apple-style-span" style="font-size: 13px; -webkit-border-horizontal-spacing: 2px; -webkit-border-vertical-spacing: 2px; ">gcc-4.3.4.</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">
<div style="color: rgb(0, 0, 0);
-webkit-border-horizontal-spacing: 0px;
-webkit-border-vertical-spacing: 0px; font-size: medium; "><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">Andrew did great
work on implementing the feature; apart from the
preprocessor changes, he added support for them in the
preprocessing record and serialization/deserialization
support in the ASTReader/ASTWriter.</span></div>
</span><span class="Apple-style-span" style="color: rgb(51, 51,
51); font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">This is a significant
amount of code that we have to maintain and make sure it works
well with other parts of the codebase, </span><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">and I'm worried about
the maintenance burden that this deprecated feature will
impose, e.g.</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">-it reserves 1 bit out
of the IdentifierInfo object. There's only 1 bit left
currently so afterwards there will be non left to easily use
for other purposes, without doubling the </span><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">IdentifierInfo
object's size.</span></div>
</blockquote>
If anyone has suggestions on how to avoid this, I am happy to
consider reworking my patch to incorporate such an improvement.
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">-it increases the size
of the IdentifierInfo data that are stored in the serialized
AST.</span></div>
</blockquote>
I think this is unavoidable if the feature is going to be
implemented, but again suggestions on how to improve things are most
welcome.<br>
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">Although the changes
are extensive, there may be more that are needed:</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">-gcc docs say that you
can specify assertions by command-line options; I did not see
this implemented in Andrew's patches</span></div>
</blockquote>
I am aware the command-line option (-A) to assert a key-value pair
is not implemented in the patches I have submitted. I didn't write
this support yet because clang uses -A for other things and I wasn't
sure if that needed to be changed or if we were going to use a
different flag - the actual patch will be relatively small. If the
feature is accepted and guidance is provided as to what flag we
should use I am happy to write and contribute this patch
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">-the feature was not
integrated into our PCH validation, to be more specific,
validating and giving errors when assertions from PCH clash
with predefined assertions or assertions from the command-line</span></div>
</blockquote>
I do not believe there is anything that needs to be validated in
terms of PCH vs command-line vs source preprocessor assertions. You
can assert multiple values for a given key quite correctly (unlike
macros) and assertions are designed to be added and removed using
all of these features and it is not wrong to do so. Further,
asserting the same key-value pair twice is not an error and so I
don't think there needs to be validation of clashes since, by
design, these clashes are permitted.
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; "><br>
</span></div>
<div><span class="Apple-style-span" style="color: rgb(51, 51, 51);
font-size: 13px; -webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px; ">-It is not clear to me
how this feature should interact with modules.</span></div>
</blockquote>
I am not sure how these would interact since the reference
implementation, GCC, does not support modules in the versions I have
tested. Doug, do you have any thoughts on this since you have worked
on modules? Since the behavior is undefined we can probably do
whatever is easiest and makes the most sense.
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;"><br>
</span></font></div>
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;"><br>
</span></font></div>
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;">So, to recap, the
question is this, should we implement and maintain a gcc
extension that was deprecated since gcc 3 ?</span></font></div>
</blockquote>
I think clang should implement and maintain this feature because<br>
1) It's widely supported by existing compilers including GCC 4.6.X (<a class="moz-txt-link-freetext" href="http://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Preprocessor-Options.html#Preprocessor-Options">http://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Preprocessor-Options.html#Preprocessor-Options</a>),
Intel C/C++ (<a class="moz-txt-link-freetext" href="http://denali.princeton.edu/intel_cc_docs/c_ug/index.htm">http://denali.princeton.edu/intel_cc_docs/c_ug/index.htm</a>),
and ARM's toolchain
(<a class="moz-txt-link-freetext" href="http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CHDFDIDE.html">http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0491c/CHDFDIDE.html</a>)<br>
2) Preprocessor assertions are an optional extension defined in
System V Release 4 (SVR4) see the System V Interface manual Vol 3
page 270 (<a class="moz-txt-link-freetext" href="http://www.sco.com/developers/devspecs/vol3_ps.ps">http://www.sco.com/developers/devspecs/vol3_ps.ps</a>)
and so should be implemented to support SVR4 systems which have
chosen to implement this feature.<br>
3) It was clearly identified as a missing feature by FIXMEs in
Chris' original commit of PPDirectives.cpp and currently the
compiler produces only cryptic errors when this feature is
encountered.<br>
4) It is not possible to work around this feature without having to
modify the source and doing so with an automated sed or awk script
is risky and difficult to integrate into some build processes. I did
consider trying to emulate assertions using macros, but I found this
wasn't possible using only the preprocessor. Asserts and macros
exist in different namespaces and so you can define and assert the
same symbol. In many cases this is not a problem because all we want
to do is test if a macro is defined, but if the value of the macro
is important then a collision in the names would cause a problem.
The assertion tests themselves would have to be found and rewritten
using a sed or awk script which is a fragile hack rather than a
proper fix and having to run this script before a compile would
complicate the build process etc. This kind of workaround also
assumes that the source can be modified prior to compilation.<br>
5) Clang has publicly stated that it will aim for GCC compatibility
where possible and these patches make compatibility with this
feature possible.
<blockquote cite="mid:7A927C85-3360-4858-BCFF-CBFE5C8420F4@apple.com" type="cite">
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;"><br>
</span></font></div>
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;">-Argyrios</span></font></div>
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;"><br>
</span></font></div>
<div><font class="Apple-style-span" color="#333333"><span class="Apple-style-span" style="font-size: 13px;
-webkit-border-horizontal-spacing: 2px;
-webkit-border-vertical-spacing: 2px;"><br>
</span></font></div>
<div>
<div>On Dec 14, 2011, at 9:26 PM, Andrew Craik wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div>Hi Doug,<br>
<br>
Please find attached a series of 4 patches which have
incorporated your feedback on my previous attempt to
implement support for GCC-style pre-processor assertions in
Clang; the feature is now working with PCH and PCH chaining.
The patches are as follows from first to last:<br>
1) boost_usuallytinyptrvector_to_basic.patch - moves the
UsuallyPtrVector type to Basic, per your previous, and adds
an erase method to it<br>
2) pp_assert_impl.patch - reworked version of the original
patch to implement handling of asserts in the pre-processor<br>
3) pp_assert_defaults.patch - same as previous post adding
the default asserts for systems and cpus<br>
4) pp_assert_serialization.patch - adds serialization
support for assertions and all the required callbacks etc<br>
<br>
For the serialization I had to add a bit to the
IdentifierInfo to track if the identifier had asserted
values to facilitate separate deserialization of asserts and
macros. This resulted in the size of the emitted
IdentifierInfo exceeding the 2 bytes that were allocated for
it so I have increased the size as you can see in the patch.<br>
<br>
I look forward to hearing from you.<br>
<br>
Andrew<br>
<br>
On 04-Dec-11 08:33, Andrew Craik wrote:<br>
<blockquote type="cite">Hi Doug,<br>
</blockquote>
<blockquote type="cite"><br>
</blockquote>
<blockquote type="cite">Thanks for the detailed feedback on
the code style and control flow; all<br>
</blockquote>
<blockquote type="cite">your comments seem very reasonable
and I will go and rework the patch. I<br>
</blockquote>
<blockquote type="cite">will see if I can add another patch
to do the<br>
</blockquote>
<blockquote type="cite">serialization/de-serialization.<br>
</blockquote>
<blockquote type="cite"><br>
</blockquote>
<blockquote type="cite">Andrew<br>
</blockquote>
<blockquote type="cite"><br>
</blockquote>
<blockquote type="cite">On 04-Dec-11 06:32, Douglas Gregor
wrote:<br>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">On Dec 1, 2011, at 11:46 PM,
Andrew Craik wrote:<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">Hello,<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">At the LLVM Developers' Meeting
a couple of weeks ago I mentioned that I had a patch
to add support to the pre-processor to handle
GCC-style pre-processor assertions.<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">Please find attached two
patches:<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">pp_assert_impl.patch - this adds
handling of assertions to the pre-processor<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">pp_assert_default.patch - this
adds default cpu and system assertions to Targets.cpp<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">I look forward to hearing from
the community and hopefully getting these patches
integrated.<br>
</blockquote>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Thanks. A bunch of comments below,
but generally things are look very good!<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Starting with
pp_assert_impl.patch:<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Index:
clang/include/clang/Lex/Preprocessor.h<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">===================================================================<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">---
clang.orig/include/clang/Lex/Preprocessor.h<span class="Apple-tab-span" style="white-space:pre"> </span>2011-11-30
11:39:19.000000000 +1000<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+++
clang/include/clang/Lex/Preprocessor.h<span class="Apple-tab-span" style="white-space:pre"> </span>2011-12-02
12:23:53.246940900 +1000<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">@@ -15,6 +15,7 @@<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> #define
LLVM_CLANG_LEX_PREPROCESSOR_H<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> #include
"clang/Lex/MacroInfo.h"<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+#include
"clang/Lex/AssertionInfo.h"<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> #include "clang/Lex/Lexer.h"<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> #include "clang/Lex/PTHLexer.h"<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> #include
"clang/Lex/PPCallbacks.h"<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">@@ -240,6 +241,10 @@<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> /// to the actual definition
of the macro.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">
llvm::DenseMap<IdentifierInfo*, MacroInfo*>
Macros;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// Assertions - For each
IdentifierInfo that has appeared as an asserted<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// predicate, we store the
values for which it was asserted<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
llvm::DenseMap<IdentifierInfo*,
std::vector<AssertionInfo*> >
AssertedValues;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">I'm guessing that the common case
is that there is only one asserted value for each
assertion, so I suggest replacing the std::vector with a
UsuallyTinyPtrVector (which will need to be boosted from
AST to Basic).<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">@@ -440,6 +459,30 @@<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> macro_iterator
macro_begin(bool IncludeExternalMacros = true) const;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> macro_iterator macro_end(bool
IncludeExternalMacros = true) const;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// AddAssertionValue - assert
the specified identifier to the<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// set of asserted values for
the given assertion key<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ void
AddAssertionValue(IdentifierInfo *II, AssertionInfo
*AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// IsAsserted - test if the
specified identifier has been asserted as a<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// value of the given
assertion key returns true iff it has<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ bool IsAsserted(IdentifierInfo
*II, AssertionInfo *AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// RemoveAssertionValue -
remove the specified identifier from the set of<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// asserted values for the
given assertion key<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ void
RemoveAssertionValue(IdentifierInfo *II, AssertionInfo
*AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// RemoveAssertion - remove
the specified assertion and all of its asserted<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ /// values<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ void
RemoveAssertion(IdentifierInfo *II);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">A quick request here… the LLVM
coding standards clarified the naming of functions a few
months ago (<a moz-do-not-send="true" href="http://llvm.org/docs/CodingStandards.html#ll_naming">http://llvm.org/docs/CodingStandards.html#ll_naming</a>),
so please name these addAssertionValue, isAsserted, and
so on.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">The same comment applies
throughout the patch, because we want new code to follow
this convention and existing code to eventually migrate
over.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ void Destory() {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ ValueTokens.clear();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ this->~AssertionInfo();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Typo "Destory", propagated
throughout.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+/// ReadAssertionPredicate - Lex
and validate an assertion predicate, which<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+/// occurs after an #assert or
#unassert. This sets the token kind to eod and<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+/// discards the rest of the line
if the assert is invalid.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+/// isAssertUnassert is 1 if this
is due to an #assert, 2 if #unassert<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+/// directive, 0 if it is
something else<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+void
Preprocessor::ReadAssertionPredicate(Token&AssertionPredTok,<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ char isAssertUnassert) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Please use an enumeration type
rather than 0/1/2 here.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Also, why not indicate failure by
returning 'true' and success by returning 'false', as we
do elsewhere in Clang?<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ IdentifierInfo *II =
AssertionPredTok.getIdentifierInfo();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (II == 0) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ bool Invalid = false;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ std::string Spelling =
getSpelling(AssertionPredTok,&Invalid);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (Invalid) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ const IdentifierInfo&Info
= Identifiers.get(Spelling);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if
(Info.isCPlusPlusOperatorKeyword()) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Diag(AssertionPredTok,
diag::err_pp_operator_used_as_assert_predicate)<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<< Spelling;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ } else {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Diag(AssertionPredTok,
diag::err_pp_predicate_not_identifier);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // Fall through on error.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ } else if
(isAssertUnassert&& II->getPPKeywordID() ==
tok::pp_assert) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // Error if using assert as a
predicate since using it would require<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // #assert<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Diag(AssertionPredTok,
diag::err_assert_predicate_name);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ } else {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // Okay, we got a good
identifier node. Return it.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">If you did switch the return value
to 'bool', this code could be de-nested and clarified a
bit:<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">if (IdentifierInfo *II =
AssertionPredTok.getIdentifierInfo()) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> if (isAssertUnassert&&
II->getPPKeywordID() == tok::pp_assert)<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> return
Diag(AssertionPredTok, diag::err_assert_predicate_name);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"> return false;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">}<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">// diagnose non-identifiers.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+//===----------------------------------------------------------------------===//<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+// Preprocessor Assertion
Directive Handling.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+//===----------------------------------------------------------------------===//<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+void
Preprocessor::HandleAssertDirective(Token&DTok) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Token AssertionPredicateTok;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
ReadAssertionPredicate(AssertionPredicateTok, 1);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (<a moz-do-not-send="true" href="http://AssertionPredicateTok.is/">AssertionPredicateTok.is</a>(tok::eod))
{<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AssertionInfo *AI =<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
AllocateAssertionInfo(AssertionPredicateTok.getLocation());<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Token Tok;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ LexUnexpandedToken(Tok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ ReadAssertionValue(Tok, AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // add to AI<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (AI != 0) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">I find this interface to
ReadAssertionValue to be a bit odd. How about having
ReadAssertionValue lex all of the tokens it needs to,
then return an allocated AssertionInfo* on success or
null to indicate failure? It would simplify control flow
for its callers.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+void
Preprocessor::HandleUnassertDirective(Token&DTok) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Token AssertionPredicateTok;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
ReadAssertionPredicate(AssertionPredicateTok, 2);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // check to see if we read eod
which indicates an error has occurred and<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // an appropriate diagnostic
has already been generated<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (<a moz-do-not-send="true" href="http://AssertionPredicateTok.is/">AssertionPredicateTok.is</a>(tok::eod))
{<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ Token Tok;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ LexUnexpandedToken(Tok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ IdentifierInfo *II =
AssertionPredicateTok.getIdentifierInfo();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // we have a valid identifier
so we need to create an AssertionInfo to hold<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // the value to associate with
the predicate<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ assertion_iterator iter =
AssertedValues.find(II);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // we don't have an entry in
the assertion table for the predicate, this is a<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // noop so escape early<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (iter == assertion_end()) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Shouldn't we still parse the rest
of the #unassert to make sure it's well-formed? Or, at
the very least, skip to the end of the directive?<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+void
Preprocessor::ReadAssertionValue(Token&Tok,
AssertionInfo *AI) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (Tok.isNot(tok::l_paren)) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // emit diag<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ ReleaseAssertionInfo(AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI = 0;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return
DiscardUntilEndOfDirective();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">You're missing the diagnostic
here.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ while (true) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ LexUnexpandedToken(Tok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (<a moz-do-not-send="true" href="http://Tok.is/">Tok.is</a>(tok::r_paren) || <a moz-do-not-send="true" href="http://Tok.is/">Tok.is</a>(tok::eod))
{<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ break;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI->AddToken(Tok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Could you add a comment saying
that it's intentional that parentheses don't nest?It
surprised me, but I see that's the documented behavior,
and I don't want someone coming through later and
"fixing" it.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // check for the closing ), if
we don't have it relase the AssetionInfo and<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // discard the rest of the
directive<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (!Tok.is(tok::r_paren)) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ ReleaseAssertionInfo(AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI = 0;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return
DiscardUntilEndOfDirective();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Comment typo "relase". Also,
shouldn't we emit a diagnostic here?<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // check there is nothing after
the closing ), if we have anything release<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // the Assertioninfo and
idscard the rest of the directive<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ LexUnexpandedToken(Tok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (!Tok.is(tok::eod)) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ ReleaseAssertionInfo(AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI = 0;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return
DiscardUntilEndOfDirective();<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Diagnostic here?<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ while (true) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
PP.LexUnexpandedToken(PeekTok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if (<a moz-do-not-send="true" href="http://PeekTok.is/">PeekTok.is</a>(tok::r_paren)
|| <a moz-do-not-send="true" href="http://PeekTok.is/">PeekTok.is</a>(tok::eod))
{<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ break;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI->AddToken(PeekTok);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Same request for a comment about
non-nesting parens.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // check for the closing ),
if we don't have it relase the AssertionInfo<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ // and discard the rest of
the directive<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ if
(!PeekTok.is(tok::r_paren)) {<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ PP.Diag(PredicateTok,
diag::err_pp_expr_bad_token_start_expr);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+
PP.ReleaseAssertionInfo(AI);<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ AI = 0;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ return true;<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">+ }<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">"relase" typo in comment.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">This patch is looking really good.
One non-trivial piece that's still missing is that the
AssertedValues map will need to be serialized to the AST
file and (lazily) deserialized, so that this feature
works with precompiled headers.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">pp_assert_default.patch looks good
to me.<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite">Thanks for working on this!<br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><br>
</blockquote>
</blockquote>
<blockquote type="cite">
<blockquote type="cite"><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug<br>
</blockquote>
</blockquote>
<blockquote type="cite">_______________________________________________<br>
</blockquote>
<blockquote type="cite">cfe-commits mailing list<br>
</blockquote>
<blockquote type="cite"><a moz-do-not-send="true" href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
</blockquote>
<blockquote type="cite"><a moz-do-not-send="true" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote>
<span><pp_assert_defaults.patch></span><span><pp_assert_impl.patch></span><span><pp_assert_serialization.patch></span><span><boost_usuallytinyptrvector_to_basic.patch></span>_______________________________________________<br>
cfe-commits mailing list<br>
<a moz-do-not-send="true" href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div>
</blockquote>
</div>
<br>
</blockquote>
</div>
</blockquote></div><br></body></html>