<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div></div><br><div><div>On Jul 29, 2014, at 3:15 PM, Tyler Nowicki <<a href="mailto:tnowicki@apple.com">tnowicki@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi Aaron,</div><div><br></div><div>I’ve cut another piece off the constant expression patch. This piece adds the state variable to the loop hint attribute. Would you be able tor review it?</div><div><br></div><div>Parts of this patch may seem a little overkill, they are necessary to support constant expressions. Specifically, the way the argument to a loop hint pragma is handled. The single token argument is replaced by an array of tokens which either give a keyword (single token) or a constant expression (multiple tokens). We need to have these cases separated out to pass the hint to the SemaStmtAttr handler. Consequently validation of the keywords must move from SemaStmtAttr handler to the loop hint annotation token handler. It makes the token handler a bit more complicated and the SemaStmtAttr a bit simpler.</div><div><br></div><div>A couple of new tests are included as well.</div><div><br></div><div>Tyler</div><div><br></div><div></div></div><span><loop_hint_attr_state-svn.patch></span><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div></div><div><br></div><br><div><div>On Jul 28, 2014, at 3:00 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Mon, Jul 28, 2014 at 5:37 PM, Tyler Nowicki <<a href="mailto:tnowicki@apple.com">tnowicki@apple.com</a>> wrote:<br><blockquote type="cite"><br><blockquote type="cite"><blockquote type="cite">Hi,<br><br>My internship finishes up this week, and Aaron informed me he won’t be able<br>to respond until next week. Would someone be able to review the patches<br>these patches so I can get them in before I leave?<br></blockquote><br>Not to make a liar of you (I really am on "vacation"), but I don't<br>want you to get stuck waiting on me. :-)<br><br>Minor comments, but with fixing them up, LGTM!<br></blockquote><br>Thanks! Sorry to drag you away from your vacation.<br></blockquote><br>No worries. :-)<br><br><blockquote type="cite"><br>I made your corrections and I will commit the patch. The only think I didn’t do was to change the raw_ostream because this patch is a lead-up to the constant expression patch. AFAIK the only way to print a constant expression is to use printPretty which requires a raw_ostream. So 'OS << value' in the this patch becomes 'value->printPretty(OS, nullptr, Policy);’ in the next.<br></blockquote><br>Ah, sounds good to me!<br><br><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Index: include/clang/Basic/Attr.td<br>===================================================================<br>--- include/clang/Basic/Attr.td (revision 214099)<br>+++ include/clang/Basic/Attr.td (working copy)<br>@@ -1815,12 +1815,6 @@<br> llvm_unreachable("Unhandled LoopHint option.");<br> }<br><br>- static StringRef getValueName(int Value) {<br>- if (Value)<br>- return "enable";<br>- return "disable";<br>- }<br>-<br></blockquote><br>Good catch here, but will this conflict with Mark's upcoming patch? I<br>don't think it will, but better to ask now. ;-)<br></blockquote><br>I don’t think so. Mark’s nounroll patch is already in.<br></blockquote><br>Phew. :-)<br><br>~Aaron</div></blockquote></div><br></div>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></body></html>