<div dir="ltr">Hi Aaron.<div>Thank you for the review. I'm attaching a new patch.</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On 7 May 2014 15:10, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
Style change -- else if should be on the same line as }<br></blockquote><div><br></div><div>Fixed.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
> --- test/SemaCXX/pragma-optimize-diagnostics.cpp (revision 0)<br>
> +++ test/SemaCXX/pragma-optimize-diagnostics.cpp (revision 0)<br>
<br>
This seems like it should be a parser test more than a sema test.<br></blockquote><div><br></div><div>Moved to the Parser directory. Also added a few comments and changed the diagnostic message (see below).</div><div> </div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">> +// - #pragma clang optimize on/off<br>

> +def err_pragma_optimize_malformed : Error<<br>
> +  "malformed argument to '#pragma clang optimize' - expected 'on' or 'off'">;<br>
<br>
Should probably be worded to use "unexpected" instead of "malformed."<br>
Perhaps: "unexpected argument '%0'; expected 'on' or 'off'" or<br>
something akin to that?<br></blockquote><div><br></div><div>Adding a %0 is problematic as sometimes the error is that the argument is missing or is not an identifier, so there's no text to display in the %0. Rather than adding another diagnostic just for that specific case, I have changed the error to "pragma clang optimize is malformed; it requires 'on' or 'off'", which is very similar to an error message a couple of lines above (the one about pragma detect_mismatch).</div>
<div>Is that satisfactory?</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
> +  /// directive if such directive has not been closed by an "on" yet. If<br>
<br>
"such directive" should be "such a directive".<br></blockquote><div><br></div><div>Fixed.</div><div> </div><div>Cheers,</div><div>    Dario Domizioli</div><div>    SN Systems - Sony Computer Entertainment Group</div>
<div><br></div><div><br></div></div></div></div>