[llvm] r283018 - Use StringRef instead of raw pointers in MCAsmInfo/MCInstrInfo APIs (NFC)
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 1 09:08:13 PDT 2016
On Sat, Oct 1, 2016 at 8:52 AM Mehdi Amini <mehdi.amini at apple.com> wrote:
> 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> 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
> 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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
>
> ==============================================================================
> --- 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.
>
Yea you are right. In that case, how hard is it to change Str to a
StringRef as well?
>
> 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?
>
Yea I suppose so. I just get worried when I see .data(). For example the
following is unsafe:
void Foo(StringRef S) {
const char *cstr = S.data();
}
void Bar() {
Foo("Test");
}
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161001/2c570e5f/attachment.html>
More information about the llvm-commits
mailing list