<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Sat, Oct 1, 2016 at 8:52 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<br></div><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="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Oct 1, 2016, at 8:08 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:</div><br class="m_5681434135962580196Apple-interchange-newline gmail_msg"></blockquote></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Fri, Sep 30, 2016 at 11:55 PM Mehdi Amini via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" 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">Author: mehdi_amini<br class="gmail_msg">Date: Sat Oct  1 01:46:33 2016<br class="gmail_msg">New Revision: 283018<br class="gmail_msg"><br class="gmail_msg">URL:<span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=283018&view=rev" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project?rev=283018&view=rev</a><br class="gmail_msg">Log:<br class="gmail_msg">Use StringRef instead of raw pointers in MCAsmInfo/MCInstrInfo APIs (NFC)<br class="gmail_msg"><br class="gmail_msg">Modified:<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/include/llvm/MC/MCAsmInfo.h<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/include/llvm/MC/MCInstrInfo.h<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/MC/MCParser/AsmLexer.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h<br class="gmail_msg">   <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>llvm/trunk/tools/llvm-objdump/MachODump.cpp<br class="gmail_msg"><br class="gmail_msg">Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h<br class="gmail_msg">URL:<span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=283018&r1=283017&r2=283018&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=283018&r1=283017&r2=283018&view=diff</a><br class="gmail_msg">==============================================================================<br class="gmail_msg">--- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)<br class="gmail_msg">+++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Sat Oct  1 01:46:33 2016<br class="gmail_msg">@@ -103,7 +103,7 @@ protected:<br class="gmail_msg"><br class="gmail_msg">   /// This indicates the comment character used by the assembler.  Defaults to<br class="gmail_msg">   /// "#"<br class="gmail_msg">-  const char *CommentString;<br class="gmail_msg">+  StringRef CommentString;<br class="gmail_msg"><br class="gmail_msg">   /// This is appended to emitted labels.  Defaults to ":"<br class="gmail_msg">   const char *LabelSuffix;<br class="gmail_msg">@@ -117,17 +117,17 @@ protected:<br class="gmail_msg">   /// This prefix is used for globals like constant pool entries that are<br class="gmail_msg">   /// completely private to the .s file and should not have names in the .o<br class="gmail_msg">   /// file.  Defaults to "L"<br class="gmail_msg">-  const char *PrivateGlobalPrefix;<br class="gmail_msg">+  StringRef PrivateGlobalPrefix;<br class="gmail_msg"><br class="gmail_msg">   /// This prefix is used for labels for basic blocks. Defaults to the same as<br class="gmail_msg">   /// PrivateGlobalPrefix.<br class="gmail_msg">-  const char *PrivateLabelPrefix;<br class="gmail_msg">+  StringRef PrivateLabelPrefix;<br class="gmail_msg"><br class="gmail_msg">   /// This prefix is used for symbols that should be passed through the<br class="gmail_msg">   /// assembler but be removed by the linker.  This is 'l' on Darwin, currently<br class="gmail_msg">   /// used for some ObjC metadata.  The default of "" meast that for this system<br class="gmail_msg">   /// a plain private symbol should be used.  Defaults to "".<br class="gmail_msg">-  const char *LinkerPrivateGlobalPrefix;<br class="gmail_msg">+  StringRef LinkerPrivateGlobalPrefix;<br class="gmail_msg"><br class="gmail_msg">   /// If these are nonempty, they contain a directive to emit before and after<br class="gmail_msg">   /// an inline assembly statement.  Defaults to "#APP\n", "#NO_APP\n"<br class="gmail_msg">@@ -468,17 +468,17 @@ public:<br class="gmail_msg">   /// printed.<br class="gmail_msg">   unsigned getCommentColumn() const { return 40; }<br class="gmail_msg"><br class="gmail_msg">-  const char *getCommentString() const { return CommentString; }<br class="gmail_msg">+  StringRef getCommentString() const { return CommentString; }<br class="gmail_msg">   const char *getLabelSuffix() const { return LabelSuffix; }<br class="gmail_msg"><br class="gmail_msg">   bool useAssignmentForEHBegin() const { return UseAssignmentForEHBegin; }<br class="gmail_msg">   bool needsLocalForSize() const { return NeedsLocalForSize; }<br class="gmail_msg">-  const char *getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }<br class="gmail_msg">-  const char *getPrivateLabelPrefix() const { return PrivateLabelPrefix; }<br class="gmail_msg">+  StringRef getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }<br class="gmail_msg">+  StringRef getPrivateLabelPrefix() const { return PrivateLabelPrefix; }<br class="gmail_msg">   bool hasLinkerPrivateGlobalPrefix() const {<br class="gmail_msg">     return LinkerPrivateGlobalPrefix[0] != '\0';<br class="gmail_msg">   }<br class="gmail_msg">-  const char *getLinkerPrivateGlobalPrefix() const {<br class="gmail_msg">+  StringRef getLinkerPrivateGlobalPrefix() const {<br class="gmail_msg">     if (hasLinkerPrivateGlobalPrefix())<br class="gmail_msg">       return LinkerPrivateGlobalPrefix;<br class="gmail_msg">     return getPrivateGlobalPrefix();<br class="gmail_msg"><br class="gmail_msg">Modified: llvm/trunk/include/llvm/MC/MCInstrInfo.h<br class="gmail_msg">URL:<span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff</a><br class="gmail_msg">==============================================================================<br class="gmail_msg">--- llvm/trunk/include/llvm/MC/MCInstrInfo.h (original)<br class="gmail_msg">+++ llvm/trunk/include/llvm/MC/MCInstrInfo.h Sat Oct  1 01:46:33 2016<br class="gmail_msg">@@ -48,9 +48,9 @@ public:<br class="gmail_msg">   }<br class="gmail_msg"><br class="gmail_msg">   /// \brief Returns the name for the instructions with the given opcode.<br class="gmail_msg">-  const char *getName(unsigned Opcode) const {<br class="gmail_msg">+  StringRef getName(unsigned Opcode) const {<br class="gmail_msg">     assert(Opcode < NumOpcodes && "Invalid opcode!");<br class="gmail_msg">-    return &InstrNameData[InstrNameIndices[Opcode]];<br class="gmail_msg">+    return StringRef(&InstrNameData[InstrNameIndices[Opcode]]);<br class="gmail_msg">   }<br class="gmail_msg"> };<br class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg">Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp<br class="gmail_msg">URL:<span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=283018&r1=283017&r2=283018&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=283018&r1=283017&r2=283018&view=diff</a><br class="gmail_msg">==============================================================================<br class="gmail_msg">--- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)<br class="gmail_msg">+++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Sat Oct  1 01:46:33 2016<br class="gmail_msg">@@ -51,7 +51,7 @@ MCSymbol *MachineBasicBlock::getSymbol()<br class="gmail_msg">   if (!CachedMCSymbol) {<br class="gmail_msg">     const MachineFunction *MF = getParent();<br class="gmail_msg">     MCContext &Ctx = MF->getContext();<br class="gmail_msg">-    const char *Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();<br class="gmail_msg">+    auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();<br class="gmail_msg">     assert(getNumber() >= 0 && "cannot get label for unreachable MBB");<br class="gmail_msg">     CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" +<br class="gmail_msg">                                           <span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span>Twine(MF->getFunctionNumber()) +<br class="gmail_msg"><br class="gmail_msg">Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp<br class="gmail_msg">URL:<span class="m_5681434135962580196Apple-converted-space gmail_msg"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff" rel="noreferrer" class="gmail_msg" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff</a><br class="gmail_msg">==============================================================================<br class="gmail_msg">--- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)<br class="gmail_msg">+++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Sat Oct  1 01:46:33 2016<br class="gmail_msg">@@ -84,8 +84,8 @@ unsigned TargetInstrInfo::getInlineAsmLe<br class="gmail_msg">     if (*Str == '\n' || strncmp(Str, MAI.getSeparatorString(),<br class="gmail_msg">                                 strlen(MAI.getSeparatorString())) == 0) {<br class="gmail_msg">       atInsnStart = true;<br class="gmail_msg">-    } else if (strncmp(Str, MAI.getCommentString(),<br class="gmail_msg">-                       strlen(MAI.getCommentString())) == 0) {<br class="gmail_msg">+    } else if (strncmp(Str, MAI.getCommentString().data(),<br class="gmail_msg">+                       MAI.getCommentString().size()) == 0) {<br class="gmail_msg"></blockquote></div></div></div></blockquote></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg">This should be "MAI.getCommentString() == Str”</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">This implies building a StringRef with Str which means that 1) you need to be sure that Str is null terminated, and 2) you’ll run strlen on Str before doing a str compare, and 3) the most important, the original comparison matches that Str *starts with* getCommentString() while your replacement is an exact match.</div><div class="gmail_msg">So the correct replacement would be `StringRef(Str).startwith(MAI.getCommentString())`, with the two drawbacks above.</div></div></div></blockquote><div>Yea you are right.  In that case, how hard is it to change Str to a StringRef as well?</div><div> </div><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="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Are all these strings guaranteed to be null terminated?  I see a lot of calling .data() </div></div></div></div></blockquote><br class="gmail_msg"></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"></div><div class="gmail_msg">Since I’m converting from raw pointer, I assume that they were till now, otherwise it would have crashed? Maybe I missed something?</div></div></blockquote><div> Yea I suppose so.  I just get worried when I see .data().  For example the following is unsafe:</div><div><br></div><div>void Foo(StringRef S) {</div><div>  const char *cstr = S.data();</div><div>}</div><div><br></div><div>void Bar() {</div><div>   Foo("Test");<br></div><div>}</div><div><br></div><div>Because while it is safe from Bar, you don't know who else might be calling Foo.  As long as you control all inputs to Foo though it's ok.  To avoid this, whenever I convert something, I've been converting everything else that depends on it too, insofar as it's practical to do it without exploding into a million file CL.</div></div></div>