<div dir="ltr">Hey,<div><br></div><div>Looking at the changes it should work, however, im not able to test it,</div><div>i was using github mirror to checkout original code and work with it.</div><div>then used llvm's steps (<a href="https://llvm.org/docs/Phabricator.html">https://llvm.org/docs/Phabricator.html</a>) to create the patch and submit.</div><div>however, the guide does not show how i take a existing revision from phabricator</div><div>i've already downloaded arc and got .arcconfig</div><div>however this tool is totally new to me, can you please assist?</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 27, 2018 at 2:36 AM Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Feb 26, 2018 at 4:27 PM Rafael Avila de Espindola <<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Rui Ueyama via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> writes:<br>
<br>
> Index: lld/include/lld/Common/Strings.h<br>
> ===================================================================<br>
> --- lld/include/lld/Common/Strings.h<br>
> +++ lld/include/lld/Common/Strings.h<br>
> @@ -10,14 +10,71 @@<br>
>  #ifndef LLD_STRINGS_H<br>
>  #define LLD_STRINGS_H<br>
><br>
> +#include "llvm/ADT/ArrayRef.h"<br>
>  #include "llvm/ADT/Optional.h"<br>
>  #include "llvm/ADT/StringRef.h"<br>
> +#include "llvm/Support/GlobPattern.h"<br>
>  #include <string><br>
> +#include <vector><br>
> +<br>
> +namespace llvm {<br>
> +class GlobPattern;<br>
> +}<br>
<br>
Why do you need both the include and the forward declaration?<br>
<br>
<br>
>  namespace lld {<br>
>  // Returns a demangled C++ symbol name. If Name is not a mangled<br>
>  // name, it returns Optional::None.<br>
>  llvm::Optional<std::string> demangleItanium(llvm::StringRef Name);<br>
> +llvm::Optional<std::string> demangleMSVC(llvm::StringRef S);<br>
<br>
Should this be in Common? When would any linker other than COFF need<br>
this?<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>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?</div></div></div><div dir="ltr"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> +std::vector<uint8_t> parseHex(llvm::StringRef S);<br>
> +bool isValidCIdentifier(llvm::StringRef S);<br>
> +<br>
> +// This is a lazy version of StringRef. String size is computed lazily<br>
> +// when it is needed. It is more efficient than StringRef to instantiate<br>
> +// if you have a string whose size is unknown.<br>
> +//<br>
> +// ELF string tables contain a lot of null-terminated strings.<br>
<br>
COFF too, no?<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div></div></blockquote></div>