[lld] r228968 - [ELF] Insert wrap symbols into a set.

[Rui Ueyama]

On Thu, Feb 12, 2015 at 6:50 PM, Shankar Easwaran <shankare at codeaurora.org>
wrote:

> Hi Rui,
>
> Unfortunately types are visible only within the class.
>
> Example :-
>
> class A {
> public:
>   typedef int MyType;
>   MyType foo();
> };
>
> MyType A::foo() {
>   return 0;
> }
>
> int main() {
>   A a;
>   a.foo();
> }
>
> 1.cpp:7:1: error: unknown type name 'MyType'; did you mean 'A::MyType'?
> MyType A::foo() {
>
> I wanted to only return a range than giving full access to the data
> structure outside the class.
>

You are not going to give a full access. I suggested returning a const
reference.

Actually I think returning a range for a set is bad. The whole point of
using set here is because it's a set -- you want to check whether a symbol
is a member of the set or not. Because you return only the iterator, you
cannot call count() on it. You made O(1) to O(n).

I don't think we need something tricky here. Let's just return a const
reference to the field.


> Shankar Easwaran
>
>
> On 2/12/2015 5:55 PM, Rui Ueyama wrote:
>
>> On Thu, Feb 12, 2015 at 2:37 PM, Shankar Easwaran <
>> shankare at codeaurora.org>
>> wrote:
>>
>>  Author: shankare
>>> Date: Thu Feb 12 16:37:27 2015
>>> New Revision: 228968
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=228968&view=rev
>>> Log:
>>> [ELF] Insert wrap symbols into a set.
>>>
>>> Symbols specified by --wrap was being inserted into a vector, change this
>>> to
>>> insert into a set, so that we have unique entries.
>>>
>>> Modified:
>>>      lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
>>>      lld/trunk/lib/ReaderWriter/ELF/ELFLinkingContext.cpp
>>>      lld/trunk/test/elf/wrap.test
>>>
>>> Modified: lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/
>>> ELFLinkingContext.h?rev=228968&r1=228967&r2=228968&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h (original)
>>> +++ lld/trunk/include/lld/ReaderWriter/ELFLinkingContext.h Thu Feb 12
>>> 16:37:27 2015
>>> @@ -24,6 +24,7 @@
>>>   #include "llvm/Support/ELF.h"
>>>   #include <map>
>>>   #include <memory>
>>> +#include <set>
>>>
>>>   namespace lld {
>>>   class DefinedAtom;
>>> @@ -47,6 +48,7 @@ public:
>>>
>>>   class ELFLinkingContext : public LinkingContext {
>>>   public:
>>> +  typedef std::set<StringRef>::iterator StringRefSetIterT;
>>>
>>>  You defined StringSetIterT and
>>
>>
>>      /// \brief The type of ELF executable that the linker
>>>     /// creates.
>>> @@ -305,7 +307,7 @@ public:
>>>     // --wrap option.
>>>     void addWrapForSymbol(StringRef);
>>>
>>> -  StringRefVector wrapCalls() const;
>>> +  range<std::set<StringRef>::iterator> wrapCalls() const;
>>>
>>>   private:
>>>     ELFLinkingContext() LLVM_DELETED_FUNCTION;
>>> @@ -346,7 +348,7 @@ protected:
>>>     StringRef _soname;
>>>     StringRefVector _rpathList;
>>>     StringRefVector _rpathLinkList;
>>> -  StringRefVector _wrapCalls;
>>> +  std::set<StringRef> _wrapCalls;
>>>     std::map<std::string, uint64_t> _absoluteSymbols;
>>>     llvm::StringSet<> _dynamicallyExportedSymbols;
>>>     std::vector<std::unique_ptr<script::Parser>> _scripts;
>>>
>>> Modified: lld/trunk/lib/ReaderWriter/ELF/ELFLinkingContext.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/
>>> ELFLinkingContext.cpp?rev=228968&r1=228967&r2=228968&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/lib/ReaderWriter/ELF/ELFLinkingContext.cpp (original)
>>> +++ lld/trunk/lib/ReaderWriter/ELF/ELFLinkingContext.cpp Thu Feb 12
>>> 16:37:27 2015
>>> @@ -258,10 +258,11 @@ std::string ELFLinkingContext::demangle(
>>>
>>>   // Support --wrap option.
>>>   void ELFLinkingContext::addWrapForSymbol(StringRef symbol) {
>>> -  _wrapCalls.push_back(symbol);
>>> +  _wrapCalls.insert(symbol);
>>>   }
>>>
>>> -ELFLinkingContext::StringRefVector ELFLinkingContext::wrapCalls()
>>> const {
>>> +range<ELFLinkingContext::StringRefSetIterT>
>>>
>>>  use that here with range<>. Is this needed? Why don't you just return
>> const
>> std::set<StringRef> & from this function?
>>
>> +ELFLinkingContext::wrapCalls() const {
>>
>>>     return _wrapCalls;
>>>   }
>>>
>>>
>>> Modified: lld/trunk/test/elf/wrap.test
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/
>>> wrap.test?rev=228968&r1=228967&r2=228968&view=diff
>>>
>>> ============================================================
>>> ==================
>>> --- lld/trunk/test/elf/wrap.test (original)
>>> +++ lld/trunk/test/elf/wrap.test Thu Feb 12 16:37:27 2015
>>> @@ -26,9 +26,9 @@
>>>   #RUN: yaml2obj -format=elf -docnum 2 %s -o %t.wrapfoo.o
>>>   #RUN: yaml2obj -format=elf -docnum 3 %s -o %t.realfoo.o
>>>   #RUN: lld -flavor gnu -target x86_64 %t.main.o %t.wrapfoo.o
>>> %t.realfoo.o \
>>> -#RUN: --wrap foo --noinhibit-exec --output-filetype=yaml -o %t2.out
>>> +#RUN: --wrap foo --wrap foo --noinhibit-exec --output-filetype=yaml -o
>>> %t2.out
>>>   #RUN: lld -flavor gnu -target x86_64 %t.main.o %t.wrapfoo.o  \
>>> -#RUN: --wrap foo --noinhibit-exec --output-filetype=yaml -o
>>> %t2.out.undef
>>> 2>&1 |  \
>>> +#RUN: --wrap foo --wrap foo --noinhibit-exec --output-filetype=yaml -o
>>> %t2.out.undef 2>&1 |  \
>>>   #RUN: FileCheck %s -check-prefix=CHECKUNDEF
>>>   #CHECKWRAP:  - name:            main
>>>   #CHECKWRAP:    references:
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
> by the Linux Foundation
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150212/50d4320b/attachment.html>