<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 22, 2016, at 2:44 PM, Rui Ueyama <<a href="mailto:ruiu@google.com" class="">ruiu@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">LGTM with the following style change.<div class=""><br class=""></div><div class="">Instead of adding backslashes and double-quotes, we can use single-quotes to quote a string.</div><div class=""><br class=""></div><div class="">For example, instead of</div><div class=""><br class=""></div><div class="">RUN: echo "GROUP(\""%t2.so"\" \""%t3.so"\" \""%t4.so"\")" > %t.script<br class=""></div><div class=""><br class=""></div><div class="">we can write like this</div><div class=""><br class=""></div><div class="">RUN: echo 'GROUP("' %t2.so '"" "' %t3.so '" "' %t4.so '")" > %t.script</div></div></div></blockquote>Thats much better! Thanks for the suggestion.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">Thank you for doing this!</div></div></div></blockquote>No problem :) Thanks for the quick review.</div><div><br class=""></div><div>Will commit with those changes.</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, Jan 22, 2016 at 2:34 PM, Pete Cooper <span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="">Hi Rui<div class=""><br class=""></div><div class="">I looked in to relative paths. Unfortunately relative paths don’t get generated by lit. When you do %t for example, you get the absolute path to that temp file.</div><div class=""><br class=""></div><div class="">Attached is a patch which therefore goes the other route, and wraps all the paths in quotes. Feedback much appreciated.</div><div class=""><br class=""></div><div class="">Also, I did find a bug in tokenize with this change. Turns out that the call to substr takes the length of the substring as the second argument, not the end of the range. So we need to subtract 1 as we start from an index of 1. The test changes here are hopefully more than sufficient to ensure that doesn’t regress, given the number of quotes I had to add.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Pete</div><div class=""><br class=""></div><div class=""></div></div><br class=""><div style="word-wrap:break-word" class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Jan 22, 2016, at 1:11 PM, Pete Cooper via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class=""><div class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">Hi Rui</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><blockquote type="cite" class=""><div class="">On Jan 22, 2016, at 1:06 PM, Rui Ueyama <<a href="mailto:ruiu@google.com" target="_blank" class="">ruiu@google.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><div class="">Hi Pete,</div><div class=""><br class=""></div><div class="">Looks like both ld and gold don't think that @ is a valid character in a token, so I think we shouldn't, too.</div></div></div></blockquote>Ah, thanks for testing that.<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><pre class="">$ ld -script <(echo 'SEARCH_DIR( /Volumes/Data/apple-lld-stage1@2/ )')
ld:/dev/fd/63:1: ignoring invalid character `@' in expression
ld:/dev/fd/63:1: syntax error</pre><pre class=""><font face="arial, sans-serif" class=""><span style="white-space:normal" class="">Do you think you can fix the tests by using relative path instead of absolute path?</span></font></pre></div></div></div></blockquote>Yep, i’ll try do that now and send out a patch.</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><br class=""></div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">Thanks,</div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">Pete<br class=""><blockquote type="cite" class=""><div class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, Jan 22, 2016 at 12:48 PM, Pete Cooper<span class=""> </span><span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span><span class=""> </span>wrote:<br class=""><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">Hi Rui, Rafael<br class=""><br class="">I have a jenkins bot building lld. Due to a plugin called ‘shared workspaces’ its common to have @’s in paths.<br class=""><br class="">This bot is then failing on 3 ELF tests (ELF/as-needed.s, ELF/linkerscript.s, ELF/linkerscript2.s)<br class=""><br class="">I’ve diagnosed the issue on linkerscript.s and its down to @ not being in the set of unquoted tokens in LinkerScript::tokenize.<br class=""><br class="">I’d like to propose a patch, but first I’d like to double check what the preferred solution is. I think its either:<br class="">- Add @ to the allowable characters in a token, or<br class="">- Change the test to quote the path<br class=""><br class="">Which would you prefer?<br class=""><br class="">Thanks,<br class="">Pete<br class=""><br class="">PS, the linkerscipt.s script contains this entry which is the cause of the failure: "SEARCH_DIR( /Volumes/Data/apple-lld-stage1@2/build/tools/lld/test/ELF/Output/linkerscript2.s.tmp.dir )"</blockquote></div><br class=""></div></div></blockquote></div><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">_______________________________________________</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">llvm-commits mailing list</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><a href="mailto:llvm-commits@lists.llvm.org" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank" class="">llvm-commits@lists.llvm.org</a><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" style="font-family:Helvetica;font-size:12px;font-style:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></div></blockquote></div><br class=""></div></div><br class=""></blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></body></html>