[PATCH] D43788: Merge {COFF, ELF}/Strings.cpp to Common/Strings.cpp.

[Hagai Cohen via llvm-commits]

Hey,

Ok, so turns out i needed to checkout the full project repo and not just
LLVM+LLD repos separately as i did.
After checking out the full repo (http://llvm.org/git/) i could use arc
patch to apply the patch and test it.
Patch is working well, i was able to compile and use lld :)


On Tue, Feb 27, 2018 at 12:56 PM Hagai Cohen <hagai.co at gmail.com> wrote:

> Hey,
>
> Looking at the changes it should work, however, im not able to test it,
> i was using github mirror to checkout original code and work with it.
> then used llvm's steps (https://llvm.org/docs/Phabricator.html) to create
> the patch and submit.
> however, the guide does not show how i take a existing revision from
> phabricator
> i've already downloaded arc and got .arcconfig
> however this tool is totally new to me, can you please assist?
>
> On Tue, Feb 27, 2018 at 2:36 AM Rui Ueyama <ruiu at google.com> wrote:
>
>> On Mon, Feb 26, 2018 at 4:27 PM Rafael Avila de Espindola <
>> rafael.espindola at gmail.com> wrote:
>>
>>> Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:
>>>
>>> > Index: lld/include/lld/Common/Strings.h
>>> > ===================================================================
>>> > --- lld/include/lld/Common/Strings.h
>>> > +++ lld/include/lld/Common/Strings.h
>>> > @@ -10,14 +10,71 @@
>>> >  #ifndef LLD_STRINGS_H
>>> >  #define LLD_STRINGS_H
>>> >
>>> > +#include "llvm/ADT/ArrayRef.h"
>>> >  #include "llvm/ADT/Optional.h"
>>> >  #include "llvm/ADT/StringRef.h"
>>> > +#include "llvm/Support/GlobPattern.h"
>>> >  #include <string>
>>> > +#include <vector>
>>> > +
>>> > +namespace llvm {
>>> > +class GlobPattern;
>>> > +}
>>>
>>> Why do you need both the include and the forward declaration?
>>>
>>>
>>> >  namespace lld {
>>> >  // Returns a demangled C++ symbol name. If Name is not a mangled
>>> >  // name, it returns Optional::None.
>>> >  llvm::Optional<std::string> demangleItanium(llvm::StringRef Name);
>>> > +llvm::Optional<std::string> demangleMSVC(llvm::StringRef S);
>>>
>>> Should this be in Common? When would any linker other than COFF need
>>> this?
>>>
>>
>> Technically it is used only by COFF, but since these functions are so
>> small that I think that putting everything in one file doesn't hurt. But
>> don't you like it?
>>
>> > +std::vector<uint8_t> parseHex(llvm::StringRef S);
>>> > +bool isValidCIdentifier(llvm::StringRef S);
>>> > +
>>> > +// This is a lazy version of StringRef. String size is computed lazily
>>> > +// when it is needed. It is more efficient than StringRef to
>>> instantiate
>>> > +// if you have a string whose size is unknown.
>>> > +//
>>> > +// ELF string tables contain a lot of null-terminated strings.
>>>
>>> COFF too, no?
>>>
>>> Cheers,
>>> Rafael
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180228/5b038464/attachment.html>