<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Eli,<div><br></div><div>Thanks for your quick review! </div><div><br></div><div>I made the changes in <a href="http://llvm.org/viewvc/llvm-project?rev=68107&view=rev">http://llvm.org/viewvc/llvm-project?rev=68107&view=rev.</a></div><div><br></div><div>Only two minor comments below...</div><div><br><div><div>On Mar 30, 2009, at 8:09 PM, Eli Friedman wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>On Mon, Mar 30, 2009 at 4:46 PM, Steve Naroff <<a href="mailto:snaroff@apple.com">snaroff@apple.com</a>> wrote:<br><blockquote type="cite">+ // Check UCN constraints (C99 6.4.3p2)<br></blockquote><blockquote type="cite">+ if ((UcnVal < 0xa0 &&<br></blockquote><blockquote type="cite">+ (UcnVal != 0x24 && UcnVal != 0x40 && UcnVal != 0x60 )) // $, @, `<br></blockquote><blockquote type="cite">+ || (UcnVal >= 0xD800 && UcnVal <= 0xDFFF)) {<br></blockquote><blockquote type="cite">+ PP.Diag(Loc, diag::err_ucn_escape_invalid);<br></blockquote><blockquote type="cite">+ HadError = 1;<br></blockquote><blockquote type="cite">+ return;<br></blockquote><blockquote type="cite">+ }<br></blockquote><br>This should also check that UcnVal is less that 0x110000.<br><br><blockquote type="cite">+ // Now that we've parsed/checked the UCN, we convert from UTF32->UTF8.<br></blockquote><blockquote type="cite">+ // The conversion below was inspired by:<br></blockquote><blockquote type="cite">+ // <a href="http://www.unicode.org/Public/PROGRAMS/CVTUTF/ConvertUTF.c">http://www.unicode.org/Public/PROGRAMS/CVTUTF/ConvertUTF.c</a><br></blockquote><blockquote type="cite">+ // First, we determine how many bytes the result will require.<br></blockquote><blockquote type="cite">+ typedef unsigned char UTF8;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ unsigned short bytesToWrite = 0;<br></blockquote><blockquote type="cite">+ if (UcnVal < (UTF32)0x80)<br></blockquote><blockquote type="cite">+ bytesToWrite = 1;<br></blockquote><blockquote type="cite">+ else if (UcnVal < (UTF32)0x800)<br></blockquote><blockquote type="cite">+ bytesToWrite = 2;<br></blockquote><blockquote type="cite">+ else if (UcnVal < (UTF32)0x10000)<br></blockquote><blockquote type="cite">+ bytesToWrite = 3;<br></blockquote><blockquote type="cite">+ else<br></blockquote><blockquote type="cite">+ bytesToWrite = 4;<br></blockquote><blockquote type="cite">+<br></blockquote><blockquote type="cite">+ // If the buffer isn't big enough, bail.<br></blockquote><blockquote type="cite">+ if ((ResultBuf + bytesToWrite) >= ResultBufEnd) {<br></blockquote><blockquote type="cite">+ PP.Diag(Loc, diag::err_ucn_escape_too_big);<br></blockquote><blockquote type="cite">+ HadError = 1;<br></blockquote><blockquote type="cite">+ return;<br></blockquote><blockquote type="cite">+ }<br></blockquote><br>This should be impossible: the largest UTF-8 character is 4 bytes, and<br>the smallest escape is 6 bytes.<br><br><blockquote type="cite">+ static const UTF8 firstByteMark[7] = {<br></blockquote><blockquote type="cite">+ 0x00, 0x00, 0xC0, 0xE0, 0xF0, 0xF8, 0xFC<br></blockquote><blockquote type="cite">+ };<br></blockquote><br>The last 3 elements of this array are unused.</div></blockquote><div><br></div>I believe only the last 2 elements are unused.</div><div><br><blockquote type="cite"><div><br><br><blockquote type="cite">+ printf("%s (%d)\n", "hello \u2192 \u2603 \u2190 world", sizeof("hello \u2192 \u2603 \u2190 world"));<br></blockquote><blockquote type="cite">+ printf("%s (%d)\n", "\U00010400\U0001D12B", sizeof("\U00010400\U0001D12B"));<br></blockquote><br>It would be nice to add a compile-time test, like "int a[sizeof("hello<br>\u2192 \u2603 \u2190 world") == 24 ? 1 : -1];".<br><br>I'm assuming you're aware this doesn't handle wide strings correctly...</div></blockquote><div><br></div>Yes. I added a FIXME for now...</div><div><br></div><div>snaroff</div><div><br><blockquote type="cite"><div><br><br>-Eli<br></div></blockquote></div><br></div></body></html>