[lld] r332952 - [ELF] Simplify. NFC

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Tue May 22 14:55:11 PDT 2018


On Tue, May 22, 2018 at 12:29 PM Fāng-ruì Sòng <maskray at google.com> wrote:

> OK.
>
> if (Name.startswith(Prefix))
>   DoSomethingWith(Name.substr(Prefix.size())))
>
> should be acceptable. What if an integer constant is used?
>
> if (Name.startswith("__start_"))
>   DoSomethingWith(Name.substr(8)))
>

I can't speak for everybody, but I do accept that as long as it is obvious
what you are trying to do with the code.


> On Tue, May 22, 2018 at 9:02 AM Rui Ueyama <ruiu at google.com> wrote:
>
>> This is somewhat arguable whether this simplifies the code or not because
>> in most (perhaps all?) places in lld, StringRefs are handled as an
>> immutable (or initialize-once) object. We don't usually use StringRef's
>> member functions that mutates the internal state of the object. If you read
>> lld's code, I think you'd notice that we usually create a new StringRef
>> object when we need a substring, even if it seems a bit inefficient. I
>> believe that makes the code easier to understand as mutating an object
>> sometimes creates confusion.
>>
>> On Mon, May 21, 2018 at 11:31 PM Fangrui Song via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: maskray
>>> Date: Mon May 21 23:28:00 2018
>>> New Revision: 332952
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=332952&view=rev
>>> Log:
>>> [ELF] Simplify. NFC
>>>
>>> Modified:
>>>     lld/trunk/ELF/LTO.cpp
>>>
>>> Modified: lld/trunk/ELF/LTO.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/LTO.cpp?rev=332952&r1=332951&r2=332952&view=diff
>>>
>>> ==============================================================================
>>> --- lld/trunk/ELF/LTO.cpp (original)
>>> +++ lld/trunk/ELF/LTO.cpp Mon May 21 23:28:00 2018
>>> @@ -155,8 +155,8 @@ BitcodeCompiler::BitcodeCompiler() {
>>>    for (Symbol *Sym : Symtab->getSymbols()) {
>>>      StringRef Name = Sym->getName();
>>>      for (StringRef Prefix : {"__start_", "__stop_"})
>>> -      if (Name.startswith(Prefix))
>>> -        UsedStartStop.insert(Name.substr(Prefix.size()));
>>> +      if (Name.consume_front(Prefix))
>>> +        UsedStartStop.insert(Name);
>>>    }
>>>  }
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>
> --
> 宋方睿
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180522/0e3322be/attachment.html>


More information about the llvm-commits mailing list