<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 25, 2017 at 4:46 PM, Rafael Avila de Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- llvm/lib/Object/<wbr>COFFImportFile.cpp<br>
> +++ llvm/lib/Object/<wbr>COFFImportFile.cpp<br>
> @@ -516,10 +516,9 @@<br>
>          OF.createShortImport(*Name, E.Ordinal, ImportType, NameType));<br>
>    }<br>
><br>
> -  std::pair<StringRef, std::error_code> Result =<br>
> -      writeArchive(Path, Members, /*WriteSymtab*/ true, object::Archive::K_GNU,<br>
> -                   /*Deterministic*/ true, /*Thin*/ false);<br>
> -<br>
> +  std::pair<StringRef, std::error_code> Result = writeArchive(<br>
> +      Path, Members, /*WriteSymtab=*/true, /*WriteObjPaths=*/true,<br>
> +      object::Archive::K_GNU, /*Deterministic=*/true, /*Thin=*/false);<br>
<br>
This changes WriteObjPaths for COFF import files. Is that intentional?<br></blockquote><div><br></div><div>The names we invent for import files don't have path separators in them, so there should be no behavior change either way here. By passing true, we don't needlessly run sys::path::filename on them. Are you suggesting that maybe it will cause us to not use the short strtab format? I guess I'll change it back. Most DLL names are < 16 characters.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> -static bool useStringTable(bool Thin, StringRef Name) {<br>
> -  return Thin || Name.size() >= 16;<br>
<span class="gmail-">> +static bool useStringTable(bool WriteObjPaths, StringRef Name) {<br>
> +  // Force the use of the string table if we're writing full paths.<br>
> +  return WriteObjPaths || Name.size() >= 16;<br>
<br>
</span>I think you want to keep Thin in here. That is a format difference of<br>
thin archives:<br>
<br>
<a href="https://sourceware.org/ml/binutils/2008-03/msg00150.html" rel="noreferrer" target="_blank">https://sourceware.org/ml/<wbr>binutils/2008-03/msg00150.html</a></blockquote><div><br></div><div>Thin should imply WriteObjPaths. I added an assertion to that effect, but I can remove it and thread Thin back here as well if you like.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">> zturner added a comment.<br>><br>> Why do we even need `WriteObjPaths`?  Can't we just standardize on `true` for writing, but still support `false` for reading?<br>><br><br></span>That is an interesting idea. Can you measure what the impact in on a<br>clang build?<br></blockquote><div><br></div><div>I think if people care about the size of their static archives they should use thin archives, which always have long relative paths to the object files. It doesn't seem worth deviating from binutils ar here without a really good reason.</div></div></div></div>