<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 Oct 1, 2016, at 9:08 AM, Zachary Turner <<a href="mailto:zturner@google.com" class="">zturner@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: 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;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Sat, Oct 1, 2016 at 8:52 AM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="">mehdi.amini@apple.com</a>> wrote:<br class=""></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;"><div class="gmail_msg" style="word-wrap: break-word;"><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="gmail_msg m_5681434135962580196Apple-interchange-newline"></blockquote></div></div><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg" 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;"><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="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/include/llvm/MC/MCAsmInfo.h<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/include/llvm/MC/MCInstrInfo.h<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/MC/MCParser/AsmLexer.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </span>llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h<br class="gmail_msg"> <span class="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </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="gmail_msg m_5681434135962580196Apple-converted-space"> </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 class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg" 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;"><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 class="">Yea you are right. In that case, how hard is it to change Str to a StringRef as well?</div><div class=""> </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;"><div class="gmail_msg" style="word-wrap: break-word;"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg" 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;"><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 class="gmail_msg" style="word-wrap: break-word;"><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 class=""> Yea I suppose so. I just get worried when I see .data(). For example the following is unsafe:</div><div class=""><br class=""></div><div class="">void Foo(StringRef S) {</div><div class=""> const char *cstr = S.data();</div><div class="">}</div><div class=""><br class=""></div><div class="">void Bar() {</div><div class=""> Foo("Test");<br class=""></div><div class="">}</div><div class=""><br class=""></div><div class="">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></div></blockquote><br class=""></div><div>Right, I still have over 20 patches in my queue, trying to propagate StringRef over the codebase. I’m validating them one-by-one right now (upgrading clang/lldb/etc. at the same time).</div><div><br class=""></div><div>— </div><div>Mehdi</div><div><br class=""></div><br class=""></body></html>