<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-05-21 10:59 GMT-07:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:<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"><span class=""><br>
> On 2015 May 20, at 14:57, Alex L <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>> wrote:<br>
><br>
> I would like to resubmit the reverted commit now that I committed the AsmParser change in r237833.<br>
><br>
> I've attached an updated patch and a smaller patch that shows the change between the reverted commit and the<br>
> updated patch.<br>
><br>
> The updated patch adds a terminating null character to the LLVM IR sourced that's passed from MIRParser<br>
> to LLParser.<br>
><br>
<br>
</span>I don't think you should make a new copy of the string here.  Instead,<br>
null-terminate the source string when it's created.<br>
<br>
Here's the code that creates the source string:<br>
<br>
    case Token::TK_BlockScalar: {<br>
      getNext();<br>
      StringRef StrCopy = StringRef(T.Value).copy(NodeAllocator);<br>
      return new (NodeAllocator)<br>
          BlockScalarNode(stream.CurrentDoc, AnchorInfo.Range.substr(1),<br>
                          TagInfo.Range, StrCopy, T.Range);<br>
    }<br>
<br>
Just change this code to:<br>
<br>
    case Token::TK_BlockScalar: {<br>
      getNext();<br>
      StringRef NullStr(T.Value.c_str(), T.Value.length() + 1);<br>
      StringRef StrCopy = NullStr.copy(NodeAllocator).drop_back();<br>
      return new (NodeAllocator)<br>
          BlockScalarNode(stream.CurrentDoc, AnchorInfo.Range.substr(1),<br>
                          TagInfo.Range, StrCopy, T.Range);<br>
    }<br>
<br>
You can commit this ahead of time and write a unit test for it, then<br>
commit your previously LGTM'ed patch.<br></blockquote><div><br></div><div>I commit the YAML change in r237942.</div><div> </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">
<br>
(BTW, I used `std::string::c_str()` up above, which has always<br>
guaranteed a null-terminated string.  Since C++11, `std::string::data()`<br>
should be identical, but I don't know if all our standard libraries are<br>
up-to-date.)<br>
<div><div class="h5"><br>
> Thanks,<br>
> Alex<br>
><br>
> 2015-05-19 11:21 GMT-07:00 Alex Lorenz <<a href="mailto:arphaman@gmail.com">arphaman@gmail.com</a>>:<br>
> REPOSITORY<br>
>   rL LLVM<br>
><br>
> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D9616&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=pl0vei1ucGw5b4_w3GncjJZZv4gzRYJTACO01eVoY1A&s=Ie6Yf47dH4l7Aef9TxxE9ZpStFkNC8hLDWUvVaFTosQ&e=" target="_blank">http://reviews.llvm.org/D9616</a><br>
><br>
> Files:<br>
>   llvm/trunk/include/llvm/CodeGen/MIR/MIRParser.h<br>
>   llvm/trunk/include/llvm/CodeGen/Passes.h<br>
>   llvm/trunk/include/llvm/InitializePasses.h<br>
>   llvm/trunk/include/llvm/Support/YAMLTraits.h<br>
>   llvm/trunk/lib/CodeGen/CMakeLists.txt<br>
>   llvm/trunk/lib/CodeGen/LLVMBuild.txt<br>
>   llvm/trunk/lib/CodeGen/LLVMTargetMachine.cpp<br>
>   llvm/trunk/lib/CodeGen/MIR/CMakeLists.txt<br>
>   llvm/trunk/lib/CodeGen/MIR/LLVMBuild.txt<br>
>   llvm/trunk/lib/CodeGen/MIR/MIRParser.cpp<br>
>   llvm/trunk/lib/CodeGen/MIR/MIRPrinter.cpp<br>
>   llvm/trunk/lib/CodeGen/MIR/MIRPrinter.h<br>
>   llvm/trunk/lib/CodeGen/MIR/MIRPrintingPass.cpp<br>
>   llvm/trunk/lib/CodeGen/MIR/Makefile<br>
>   llvm/trunk/lib/CodeGen/Makefile<br>
>   llvm/trunk/lib/Support/YAMLTraits.cpp<br>
>   llvm/trunk/test/CodeGen/Generic/stop-after.ll<br>
>   llvm/trunk/test/CodeGen/MIR/lit.local.cfg<br>
>   llvm/trunk/test/CodeGen/MIR/llvmIR.mir<br>
>   llvm/trunk/test/CodeGen/MIR/llvmIRMissing.mir<br>
>   llvm/trunk/tools/llc/llc.cpp<br>
><br>
> EMAIL PREFERENCES<br>
>   <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=pl0vei1ucGw5b4_w3GncjJZZv4gzRYJTACO01eVoY1A&s=FmWuHRcv1qFj3KBUdS14nDGlDXq3f_3pb4HGt14vfbs&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
><br>
</div></div>> <MIR.patch><MIR_Revert_vs_Update.patch><br>
<br>
</blockquote></div><br></div></div>