[llvm] r283018 - Use StringRef instead of raw pointers in MCAsmInfo/MCInstrInfo APIs (NFC)
Mehdi Amini via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 1 08:52:13 PDT 2016
> On Oct 1, 2016, at 8:08 AM, Zachary Turner <zturner at google.com> wrote:
>
>
>
> On Fri, Sep 30, 2016 at 11:55 PM Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: mehdi_amini
> Date: Sat Oct 1 01:46:33 2016
> New Revision: 283018
>
> URL: http://llvm.org/viewvc/llvm-project?rev=283018&view=rev <http://llvm.org/viewvc/llvm-project?rev=283018&view=rev>
> Log:
> Use StringRef instead of raw pointers in MCAsmInfo/MCInstrInfo APIs (NFC)
>
> Modified:
> llvm/trunk/include/llvm/MC/MCAsmInfo.h
> llvm/trunk/include/llvm/MC/MCInstrInfo.h
> llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
> llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
> llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> llvm/trunk/lib/MC/MCParser/AsmLexer.cpp
> llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp
> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp
> llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h
> llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp
> llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp
> llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
> llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
> llvm/trunk/tools/llvm-objdump/MachODump.cpp
>
> Modified: llvm/trunk/include/llvm/MC/MCAsmInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCAsmInfo.h?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCAsmInfo.h (original)
> +++ llvm/trunk/include/llvm/MC/MCAsmInfo.h Sat Oct 1 01:46:33 2016
> @@ -103,7 +103,7 @@ protected:
>
> /// This indicates the comment character used by the assembler. Defaults to
> /// "#"
> - const char *CommentString;
> + StringRef CommentString;
>
> /// This is appended to emitted labels. Defaults to ":"
> const char *LabelSuffix;
> @@ -117,17 +117,17 @@ protected:
> /// This prefix is used for globals like constant pool entries that are
> /// completely private to the .s file and should not have names in the .o
> /// file. Defaults to "L"
> - const char *PrivateGlobalPrefix;
> + StringRef PrivateGlobalPrefix;
>
> /// This prefix is used for labels for basic blocks. Defaults to the same as
> /// PrivateGlobalPrefix.
> - const char *PrivateLabelPrefix;
> + StringRef PrivateLabelPrefix;
>
> /// This prefix is used for symbols that should be passed through the
> /// assembler but be removed by the linker. This is 'l' on Darwin, currently
> /// used for some ObjC metadata. The default of "" meast that for this system
> /// a plain private symbol should be used. Defaults to "".
> - const char *LinkerPrivateGlobalPrefix;
> + StringRef LinkerPrivateGlobalPrefix;
>
> /// If these are nonempty, they contain a directive to emit before and after
> /// an inline assembly statement. Defaults to "#APP\n", "#NO_APP\n"
> @@ -468,17 +468,17 @@ public:
> /// printed.
> unsigned getCommentColumn() const { return 40; }
>
> - const char *getCommentString() const { return CommentString; }
> + StringRef getCommentString() const { return CommentString; }
> const char *getLabelSuffix() const { return LabelSuffix; }
>
> bool useAssignmentForEHBegin() const { return UseAssignmentForEHBegin; }
> bool needsLocalForSize() const { return NeedsLocalForSize; }
> - const char *getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }
> - const char *getPrivateLabelPrefix() const { return PrivateLabelPrefix; }
> + StringRef getPrivateGlobalPrefix() const { return PrivateGlobalPrefix; }
> + StringRef getPrivateLabelPrefix() const { return PrivateLabelPrefix; }
> bool hasLinkerPrivateGlobalPrefix() const {
> return LinkerPrivateGlobalPrefix[0] != '\0';
> }
> - const char *getLinkerPrivateGlobalPrefix() const {
> + StringRef getLinkerPrivateGlobalPrefix() const {
> if (hasLinkerPrivateGlobalPrefix())
> return LinkerPrivateGlobalPrefix;
> return getPrivateGlobalPrefix();
>
> Modified: llvm/trunk/include/llvm/MC/MCInstrInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/MCInstrInfo.h (original)
> +++ llvm/trunk/include/llvm/MC/MCInstrInfo.h Sat Oct 1 01:46:33 2016
> @@ -48,9 +48,9 @@ public:
> }
>
> /// \brief Returns the name for the instructions with the given opcode.
> - const char *getName(unsigned Opcode) const {
> + StringRef getName(unsigned Opcode) const {
> assert(Opcode < NumOpcodes && "Invalid opcode!");
> - return &InstrNameData[InstrNameIndices[Opcode]];
> + return StringRef(&InstrNameData[InstrNameIndices[Opcode]]);
> }
> };
>
>
> Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Sat Oct 1 01:46:33 2016
> @@ -51,7 +51,7 @@ MCSymbol *MachineBasicBlock::getSymbol()
> if (!CachedMCSymbol) {
> const MachineFunction *MF = getParent();
> MCContext &Ctx = MF->getContext();
> - const char *Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
> + auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
> assert(getNumber() >= 0 && "cannot get label for unreachable MBB");
> CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" +
> Twine(MF->getFunctionNumber()) +
>
> Modified: llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp (original)
> +++ llvm/trunk/lib/CodeGen/TargetInstrInfo.cpp Sat Oct 1 01:46:33 2016
> @@ -84,8 +84,8 @@ unsigned TargetInstrInfo::getInlineAsmLe
> if (*Str == '\n' || strncmp(Str, MAI.getSeparatorString(),
> strlen(MAI.getSeparatorString())) == 0) {
> atInsnStart = true;
> - } else if (strncmp(Str, MAI.getCommentString(),
> - strlen(MAI.getCommentString())) == 0) {
> + } else if (strncmp(Str, MAI.getCommentString().data(),
> + MAI.getCommentString().size()) == 0) {
> This should be "MAI.getCommentString() == Str”
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.
So the correct replacement would be `StringRef(Str).startwith(MAI.getCommentString())`, with the two drawbacks above.
>
> // Stop counting as an instruction after a comment unt
> il the next
> // separator.
> atInsnStart = false;
>
> Modified: llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp (original)
> +++ llvm/trunk/lib/MC/MCDisassembler/Disassembler.cpp Sat Oct 1 01:46:33 2016
> @@ -138,7 +138,7 @@ static void emitComments(LLVMDisasmConte
> StringRef Comments = DC->CommentsToEmit.str();
> // Get the default information for printing a comment.
> const MCAsmInfo *MAI = DC->getAsmInfo();
> - const char *CommentBegin = MAI->getCommentString();
> + StringRef CommentBegin = MAI->getCommentString();
> unsigned CommentColumn = MAI->getCommentColumn();
> bool IsFirst = true;
> while (!Comments.empty()) {
>
> Modified: llvm/trunk/lib/MC/MCParser/AsmLexer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmLexer.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCParser/AsmLexer.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCParser/AsmLexer.cpp (original)
> +++ llvm/trunk/lib/MC/MCParser/AsmLexer.cpp Sat Oct 1 01:46:33 2016
> @@ -511,16 +511,16 @@ size_t AsmLexer::peekTokens(MutableArray
> }
>
> bool AsmLexer::isAtStartOfComment(const char *Ptr) {
> - const char *CommentString = MAI.getCommentString();
> + StringRef CommentString = MAI.getCommentString();
>
> - if (CommentString[1] == '\0')
> + if (CommentString.size() == 1)
> return CommentString[0] == Ptr[0];
>
> // Allow # preprocessor commments also be counted as comments for "##" cases
> if (CommentString[1] == '#')
> return CommentString[0] == Ptr[0];
>
> - return strncmp(Ptr, CommentString, strlen(CommentString)) == 0;
> + return strncmp(Ptr, CommentString.data(), CommentString.size()) == 0;
> operator ==
>
> }
>
> bool AsmLexer::isAtStatementSeparator(const char *Ptr) {
>
> Modified: llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp Sat Oct 1 01:46:33 2016
> @@ -1541,8 +1541,8 @@ unsigned HexagonInstrInfo::getInlineAsmL
> Length += MAI.getMaxInstLength();
> atInsnStart = false;
> }
> - if (atInsnStart && strncmp(Str, MAI.getCommentString(),
> - strlen(MAI.getCommentString())) == 0)
> + if (atInsnStart && strncmp(Str, MAI.getCommentString().data(),
> + MAI.getCommentString().size()) == 0)
> operator==
>
> atInsnStart = false;
> }
>
>
> Modified: llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.cpp Sat Oct 1 01:46:33 2016
> @@ -306,7 +306,7 @@ int HexagonMCInstrInfo::getMinValue(MCIn
> return 0;
> }
>
> -char const *HexagonMCInstrInfo::getName(MCInstrInfo const &MCII,
> +StringRef HexagonMCInstrInfo::getName(MCInstrInfo const &MCII,
> MCInst const &MCI) {
> return MCII.getName(MCI.getOpcode());
> }
>
> Modified: llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h (original)
> +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonMCInstrInfo.h Sat Oct 1 01:46:33 2016
> @@ -133,7 +133,7 @@ int getMaxValue(MCInstrInfo const &MCII,
> int getMinValue(MCInstrInfo const &MCII, MCInst const &MCI);
>
> // Return instruction name
> -char const *getName(MCInstrInfo const &MCII, MCInst const &MCI);
> +StringRef getName(MCInstrInfo const &MCII, MCInst const &MCI);
>
> // Return the operand index for the new value.
> unsigned short getNewValueOp(MCInstrInfo const &MCII, MCInst const &MCI);
>
> Modified: llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/Mips/Mips16InstrInfo.cpp Sat Oct 1 01:46:33 2016
> @@ -510,8 +510,8 @@ unsigned Mips16InstrInfo::getInlineAsmLe
> Length += MAI.getMaxInstLength();
> atInsnStart = false;
> }
> - if (atInsnStart && strncmp(Str, MAI.getCommentString(),
> - strlen(MAI.getCommentString())) == 0)
> + if (atInsnStart && strncmp(Str, MAI.getCommentString().data(),
> + MAI.getCommentString().size()) == 0)
> operator==
>
> atInsnStart = false;
> }
>
>
> Modified: llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp (original)
> +++ llvm/trunk/lib/Target/X86/Disassembler/X86Disassembler.cpp Sat Oct 1 01:46:33 2016
> @@ -96,7 +96,7 @@ void llvm::X86Disassembler::Debug(const
> dbgs() << file << ":" << line << ": " << s;
> }
>
> -const char *llvm::X86Disassembler::GetInstrName(unsigned Opcode,
> +StringRef llvm::X86Disassembler::GetInstrName(unsigned Opcode,
> const void *mii) {
> const MCInstrInfo *MII = static_cast<const MCInstrInfo *>(mii);
> return MII->getName(Opcode);
>
> Modified: llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp (original)
> +++ llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.cpp Sat Oct 1 01:46:33 2016
> @@ -825,7 +825,7 @@ static int getIDWithAttrMask(uint16_t* i
> * @param orig - The instruction that is not 16-bit
> * @param equiv - The instruction that is 16-bit
> */
> -static bool is16BitEquivalent(const char* orig, const char* equiv) {
> +static bool is16BitEquivalent(const char *orig, const char *equiv) {
> off_t i;
>
> for (i = 0;; i++) {
> @@ -850,7 +850,7 @@ static bool is16BitEquivalent(const char
> *
> * @param name - The instruction that is not 16-bit
> */
> -static bool is64Bit(const char* name) {
> +static bool is64Bit(const char *name) {
> off_t i;
>
> for (i = 0;; ++i) {
> @@ -1044,9 +1044,9 @@ static int getID(struct InternalInstruct
> return 0;
> }
>
> - const char *SpecName = GetInstrName(instructionIDWithREXW, miiArg);
> + auto SpecName = GetInstrName(instructionIDWithREXW, miiArg);
> // If not a 64-bit instruction. Switch the opcode.
> - if (!is64Bit(SpecName)) {
> + if (!is64Bit(SpecName.data())) {
> insn->instructionID = instructionIDWithREXW;
> insn->spec = specifierForUID(instructionIDWithREXW);
> return 0;
> @@ -1092,7 +1092,7 @@ static int getID(struct InternalInstruct
>
> const struct InstructionSpecifier *spec;
> uint16_t instructionIDWithOpsize;
> - const char *specName, *specWithOpSizeName;
> + llvm::StringRef specName, specWithOpSizeName;
>
> spec = specifierForUID(instructionID);
>
> @@ -1112,7 +1112,7 @@ static int getID(struct InternalInstruct
> specName = GetInstrName(instructionID, miiArg);
> specWithOpSizeName = GetInstrName(instructionIDWithOpsize, miiArg);
>
> - if (is16BitEquivalent(specName, specWithOpSizeName) &&
> + if (is16BitEquivalent(specName.data(), specWithOpSizeName.data()) &&
> (insn->mode == MODE_16BIT) ^ insn->prefixPresent[0x66]) {
> insn->instructionID = instructionIDWithOpsize;
> insn->spec = specifierForUID(instructionIDWithOpsize);
>
> Modified: llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h (original)
> +++ llvm/trunk/lib/Target/X86/Disassembler/X86DisassemblerDecoder.h Sat Oct 1 01:46:33 2016
> @@ -674,7 +674,7 @@ int decodeInstruction(InternalInstructio
> /// \param s The message to print.
> void Debug(const char *file, unsigned line, const char *s);
>
> -const char *GetInstrName(unsigned Opcode, const void *mii);
> +StringRef GetInstrName(unsigned Opcode, const void *mii);
>
> } // namespace X86Disassembler
> } // namespace llvm
>
> Modified: llvm/trunk/tools/llvm-objdump/MachODump.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=283018&r1=283017&r2=283018&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-objdump/MachODump.cpp?rev=283018&r1=283017&r2=283018&view=diff>
> ==============================================================================
> --- llvm/trunk/tools/llvm-objdump/MachODump.cpp (original)
> +++ llvm/trunk/tools/llvm-objdump/MachODump.cpp Sat Oct 1 01:46:33 2016
> @@ -6397,7 +6397,7 @@ static void emitComments(raw_svector_ost
> // Flush the stream before taking its content.
> StringRef Comments = CommentsToEmit.str();
> // Get the default information for printing a comment.
> - const char *CommentBegin = MAI.getCommentString();
> + StringRef CommentBegin = MAI.getCommentString();
> unsigned CommentColumn = MAI.getCommentColumn();
> bool IsFirst = true;
> while (!Comments.empty()) {
>
>
> Are all these strings guaranteed to be null terminated? I see a lot of calling .data()
Since I’m converting from raw pointer, I assume that they were till now, otherwise it would have crashed? Maybe I missed something?
—
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161001/2c4534bf/attachment.html>
More information about the llvm-commits
mailing list