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

Shankar Easwaran shankare at codeaurora.org
Fri Feb 13 09:51:36 PST 2015


On 2/13/2015 12:13 AM, Rui Ueyama wrote:
> 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.
There is no usage that I need count() for, or I cant think of an usage. 
I want only unique entries.

Shankar Easwaran

>
>> 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
>>
>>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation




More information about the llvm-commits mailing list