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

[Rafael Avila de Espindola via llvm-commits]

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?

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