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

[Hagai Cohen via llvm-commits]

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/20180227/f1ae1e73/attachment.html>