<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, May 25, 2017 at 5:07 PM Reid Kleckner <<a href="mailto:rnk@google.com">rnk@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_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>
> ===================================================================<br>
> --- llvm/lib/Object/COFFImportFile.cpp<br>
> +++ llvm/lib/Object/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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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="m_-6523650837970492341gmail-">> +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/binutils/2008-03/msg00150.html</a></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></blockquote><div><br></div><div>I guess it's possible I'm just missing something, but what I don't understand is why `Thin` can't just be the only determining factor of whether we write full paths.  Why the second variable?  AFAICT nowhere in this patch is `false` ever explicitly passed, it only takes on a `false` value implicitly when `Thin` is also false.  So just delete the variable, and only use the value of `Thin`?</div></div></div>