[lld] 476e4d6 - [lld/mac] Address review feedback and improve a comment

[Nico Weber via llvm-commits]

Author: Nico Weber
Date: 2021-06-02T10:54:53-04:00
New Revision: 476e4d65d482d75f789db092316327191f18dbcd

URL: https://github.com/llvm/llvm-project/commit/476e4d65d482d75f789db092316327191f18dbcd
DIFF: https://github.com/llvm/llvm-project/commit/476e4d65d482d75f789db092316327191f18dbcd.diff

LOG: [lld/mac] Address review feedback and improve a comment

I forgot to move the message() call around as requested in D103428
before committing that change. Move it now.

Also, improve the ordinal uniq'ing comment. I hadn't realized that the
distinct-but-identical files happen with --reproduce and not in general.

No behavior change.

Differential Revision: https://reviews.llvm.org/D103522

Added: 
    

Modified: 
    lld/MachO/DriverUtils.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/Writer.cpp

Removed: 
    


################################################################################
diff  --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index ba255f01cd60..457c3c643ae2 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -214,8 +214,6 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
       return nullptr;
     }
     file = make<DylibFile>(**result, umbrella, isBundleLoader);
-    if (config->printEachFile)
-      message(toString(file));
 
     // parseReexports() can recursively call loadDylib(). That's fine since
     // we wrote DylibFile we just loaded to the loadDylib cache via the `file`
@@ -231,8 +229,6 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
            magic == file_magic::macho_executable ||
            magic == file_magic::macho_bundle);
     file = make<DylibFile>(mbref, umbrella, isBundleLoader);
-    if (config->printEachFile)
-      message(toString(file));
 
     // parseLoadCommands() can also recursively call loadDylib(). See comment
     // in previous block for why this means we must copy `file` here.

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 2e04bb2e4833..6f71d1fb4f91 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -826,6 +826,9 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
     return;
   }
 
+  if (config->printEachFile)
+    message(toString(this));
+
   deadStrippable = hdr->flags & MH_DEAD_STRIPPABLE_DYLIB;
 
   if (!checkCompatibility(this))
@@ -900,6 +903,9 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
   compatibilityVersion = interface.getCompatibilityVersion().rawValue();
   currentVersion = interface.getCurrentVersion().rawValue();
 
+  if (config->printEachFile)
+    message(toString(this));
+
   if (!is_contained(skipPlatformChecks, dylibName) &&
       !is_contained(interface.targets(), config->platformInfo.target)) {
     error(toString(this) + " is incompatible with " +

diff  --git a/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index b9686ce0fd86..22bb20aa4604 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -705,17 +705,21 @@ template <class LP> void Writer::createLoadCommands() {
       // Several DylibFiles can have the same installName. Only emit a single
       // load command for that installName and give all these DylibFiles the
       // same ordinal.
-      // This can happen if:
+      // This can happen in several cases:
       // - a new framework could change its installName to an older
       //   framework name via an $ld$ symbol depending on platform_version
-      // - symlink (eg libpthread.tbd is a symlink to libSystem.tbd)
+      // - symlinks (for example, libpthread.tbd is a symlink to libSystem.tbd;
+      //   Foo.framework/Foo.tbd is usually a symlink to
+      //   Foo.framework/Versions/Current/Foo.tbd, where
+      //   Foo.framework/Versions/Current is usually a symlink to
+      //   Foo.framework/Versions/A)
       // - a framework can be linked both explicitly on the linker
       //   command line and implicitly as a reexport from a 
diff erent
       //   framework. The re-export will usually point to the tbd file
       //   in Foo.framework/Versions/A/Foo.tbd, while the explicit link will
-      //   usually find Foo.framwork/Foo.tbd. These are usually two identical
-      //   but distinct files (concrete example: CFNetwork.framework, reexported
-      //   from CoreServices.framework).
+      //   usually find Foo.framwork/Foo.tbd. These are usually symlinks,
+      //   but in a --reproduce archive they will be identical but distinct
+      //   files.
       // In the first case, *semantically distinct* DylibFiles will have the
       // same installName.
       int64_t &ordinal = ordinalForInstallName[dylibFile->dylibName];