[lld/mac] Address review feedback and improve a comment
authorNico Weber <thakis@chromium.org>
Wed, 2 Jun 2021 13:35:06 +0000 (09:35 -0400)
committerNico Weber <thakis@chromium.org>
Wed, 2 Jun 2021 14:54:53 +0000 (10:54 -0400)
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

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

index ba255f0..457c3c6 100644 (file)
@@ -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.
index 2e04bb2..6f71d1f 100644 (file)
@@ -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 " +
index b9686ce..22bb20a 100644 (file)
@@ -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 different
       //   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];