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

[Rui Ueyama via llvm-commits]

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