[llvm][TextAPI] Clean up minor bugs in YAML TextStub

cyndyishida · cyndyishida · commit 798bb093d893 · 2023-08-06T19:41:20.000-07:00
* Always print out maccatalyst in format * Traverse symbols via InterfaceFile symbol APIs * Properly track addition of flags. Reviewed By: ributzka Differential Revision: https://reviews.llvm.org/D144428 (cherry picked from commit 9b29de1)
diff --git a/llvm/lib/TextAPI/TextStub.cpp b/llvm/lib/TextAPI/TextStub.cpp
@@ -456,7 +456,7 @@ template <> struct MappingTraits<const InterfaceFile *> {
         ArchSet.insert(Library.getArchitectures());
 
       std::map<const Symbol *, ArchitectureSet> SymbolToArchSet;
-      for (const auto *Symbol : File->exports()) {
+      for (const auto *Symbol : File->symbols()) {
         auto Architectures = Symbol->getArchitectures();
         SymbolToArchSet[Symbol] = Architectures;
         ArchSet.insert(Architectures);
@@ -833,13 +833,10 @@ template <> struct MappingTraits<const InterfaceFile *> {
 
       auto handleSymbols =
           [](SectionList &CurrentSections,
-             InterfaceFile::const_filtered_symbol_range Symbols,
-             std::function<bool(const Symbol *)> Pred) {
+             InterfaceFile::const_filtered_symbol_range Symbols) {
             std::set<TargetList> TargetSet;
             std::map<const Symbol *, TargetList> SymbolToTargetList;
             for (const auto *Symbol : Symbols) {
-              if (!Pred(Symbol))
-                continue;
               TargetList Targets(Symbol->targets());
               SymbolToTargetList[Symbol] = Targets;
               TargetSet.emplace(std::move(Targets));
@@ -884,14 +881,9 @@ template <> struct MappingTraits<const InterfaceFile *> {
             }
           };
 
-      handleSymbols(Exports, File->exports(), [](const Symbol *Symbol) {
-        return !Symbol->isReexported();
-      });
-      handleSymbols(Reexports, File->exports(), [](const Symbol *Symbol) {
-        return Symbol->isReexported();
-      });
-      handleSymbols(Undefineds, File->undefineds(),
-                    [](const Symbol *Symbol) { return true; });
+      handleSymbols(Exports, File->exports());
+      handleSymbols(Reexports, File->reexports());
+      handleSymbols(Undefineds, File->undefineds());
     }
 
     const InterfaceFile *denormalize(IO &IO) {
@@ -937,24 +929,28 @@ template <> struct MappingTraits<const InterfaceFile *> {
 
           for (auto &sym : CurrentSection.Classes)
             File->addSymbol(SymbolKind::ObjectiveCClass, sym,
-                            CurrentSection.Targets);
+                            CurrentSection.Targets, Flag);
 
           for (auto &sym : CurrentSection.ClassEHs)
             File->addSymbol(SymbolKind::ObjectiveCClassEHType, sym,
-                            CurrentSection.Targets);
+                            CurrentSection.Targets, Flag);
 
           for (auto &sym : CurrentSection.Ivars)
             File->addSymbol(SymbolKind::ObjectiveCInstanceVariable, sym,
-                            CurrentSection.Targets);
+                            CurrentSection.Targets, Flag);
 
-          for (auto &sym : CurrentSection.WeakSymbols)
+          SymbolFlags SymFlag = (Flag == SymbolFlags::Undefined)
+                                    ? SymbolFlags::WeakReferenced
+                                    : SymbolFlags::WeakDefined;
+          for (auto &sym : CurrentSection.WeakSymbols) {
             File->addSymbol(SymbolKind::GlobalSymbol, sym,
-                            CurrentSection.Targets, SymbolFlags::WeakDefined);
+                            CurrentSection.Targets, Flag | SymFlag);
+          }
 
           for (auto &sym : CurrentSection.TlvSymbols)
             File->addSymbol(SymbolKind::GlobalSymbol, sym,
                             CurrentSection.Targets,
-                            SymbolFlags::ThreadLocalValue);
+                            Flag | SymbolFlags::ThreadLocalValue);
         }
       };
 
diff --git a/llvm/lib/TextAPI/TextStubCommon.cpp b/llvm/lib/TextAPI/TextStubCommon.cpp
@@ -82,7 +82,7 @@ void ScalarTraits<PlatformSet>::output(const PlatformSet &Values, void *IO,
     OS << "bridgeos";
     break;
   case PLATFORM_MACCATALYST:
-    OS << "iosmac";
+    OS << "maccatalyst";
     break;
   case PLATFORM_DRIVERKIT:
     OS << "driverkit";
@@ -112,6 +112,7 @@ StringRef ScalarTraits<PlatformSet>::input(StringRef Scalar, void *IO,
                       .Case("tvos", PLATFORM_TVOS)
                       .Case("bridgeos", PLATFORM_BRIDGEOS)
                       .Case("iosmac", PLATFORM_MACCATALYST)
+                      .Case("maccatalyst", PLATFORM_MACCATALYST)
                       .Case("driverkit", PLATFORM_DRIVERKIT)
                       .Default(PLATFORM_UNKNOWN);
 
diff --git a/llvm/lib/TextAPI/TextStubV5.cpp b/llvm/lib/TextAPI/TextStubV5.cpp
@@ -12,6 +12,7 @@
 #include "TextStubCommon.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/JSON.h"
+#include <utility>
 
 // clang-format off
 /*
@@ -334,9 +335,11 @@ Error collectSymbolsFromSegment(const Object *Segment, TargetsToSymbols &Result,
   if (Err)
     return Err;
 
-  SymbolFlags WeakFlag = SectionFlag | (SectionFlag == SymbolFlags::Undefined
-                                            ? SymbolFlags::WeakReferenced
-                                            : SymbolFlags::WeakDefined);
+  SymbolFlags WeakFlag =
+      SectionFlag |
+      (((SectionFlag & SymbolFlags::Undefined) == SymbolFlags::Undefined)
+           ? SymbolFlags::WeakReferenced
+           : SymbolFlags::WeakDefined);
   Err = collectFromArray(
       TBDKey::Weak, Segment, [&Result, WeakFlag](StringRef Name) {
         JSONSymbol Sym = {SymbolKind::GlobalSymbol, Name.str(), WeakFlag};
@@ -405,7 +408,8 @@ Expected<TargetsToSymbols> getSymbolSection(const Object *File, TBDKey Key,
     } else {
       MappedTargets = *TargetsOrErr;
     }
-    Result.emplace_back(std::make_pair(Targets, std::vector<JSONSymbol>()));
+    Result.emplace_back(
+        std::make_pair(std::move(MappedTargets), std::vector<JSONSymbol>()));
 
     auto *DataSection = Obj->getObject(Keys[TBDKey::Data]);
     auto *TextSection = Obj->getObject(Keys[TBDKey::Text]);
diff --git a/llvm/unittests/TextAPI/TextStubV3Tests.cpp b/llvm/unittests/TextAPI/TextStubV3Tests.cpp
@@ -495,7 +495,7 @@ TEST(TBDv3, Platform_bridgeOS) {
 TEST(TBDv3, Platform_macCatalyst) {
   static const char TBDv3PlatformiOSmac[] = "--- !tapi-tbd-v3\n"
                                             "archs: [ armv7k ]\n"
-                                            "platform: iosmac\n"
+                                            "platform: maccatalyst\n"
                                             "install-name: Test.dylib\n"
                                             "...\n";
 
diff --git a/llvm/unittests/TextAPI/TextStubV4Tests.cpp b/llvm/unittests/TextAPI/TextStubV4Tests.cpp
@@ -64,15 +64,15 @@ TEST(TBDv4, ReadFile) {
       "    objc-classes: []\n"
       "    objc-eh-types: []\n"
       "    objc-ivars: []\n"
-      "    weak-symbols: []\n"
+      "    weak-symbols: [weakReexport]\n"
       "    thread-local-symbols: []\n"
       "undefineds:\n"
       "  - targets: [ i386-macos ]\n"
       "    symbols: [ _symD ]\n"
       "    objc-classes: []\n"
       "    objc-eh-types: []\n"
       "    objc-ivars: []\n"
-      "    weak-symbols: []\n"
+      "    weak-symbols: [weakReference]\n"
       "    thread-local-symbols: []\n"
       "...\n";
 
@@ -114,16 +114,23 @@ TEST(TBDv4, ReadFile) {
   EXPECT_EQ(reexport, File->reexportedLibraries().front());
 
   ExportedSymbolSeq Exports, Reexports, Undefineds;
-  ExportedSymbol temp;
   for (const auto *Sym : File->symbols()) {
-    temp = ExportedSymbol{Sym->getKind(), std::string(Sym->getName()),
-                          Sym->isWeakDefined(), Sym->isThreadLocalValue()};
-    EXPECT_FALSE(Sym->isWeakReferenced());
-    if (Sym->isUndefined())
-      Undefineds.emplace_back(std::move(temp));
-    else
-      Sym->isReexported() ? Reexports.emplace_back(std::move(temp))
-                          : Exports.emplace_back(std::move(temp));
+    ExportedSymbol Temp =
+        ExportedSymbol{Sym->getKind(), std::string(Sym->getName()),
+                       Sym->isWeakDefined() || Sym->isWeakReferenced(),
+                       Sym->isThreadLocalValue()};
+    if (Sym->isUndefined()) {
+      EXPECT_FALSE(Sym->isWeakDefined());
+      Undefineds.emplace_back(std::move(Temp));
+    }
+    // Check that defined symbols cannot be set as weak referenced.
+    else if (Sym->isReexported()) {
+      EXPECT_FALSE(Sym->isWeakReferenced());
+      Reexports.emplace_back(std::move(Temp));
+    } else {
+      EXPECT_FALSE(Sym->isWeakReferenced());
+      Exports.emplace_back(std::move(Temp));
+    }
   }
   llvm::sort(Exports);
   llvm::sort(Reexports);
@@ -137,10 +144,12 @@ TEST(TBDv4, ReadFile) {
 
   static ExportedSymbol ExpectedReexportedSymbols[] = {
       {SymbolKind::GlobalSymbol, "_symC", false, false},
+      {SymbolKind::GlobalSymbol, "weakReexport", true, false},
   };
 
   static ExportedSymbol ExpectedUndefinedSymbols[] = {
       {SymbolKind::GlobalSymbol, "_symD", false, false},
+      {SymbolKind::GlobalSymbol, "weakReference", true, false},
   };
 
   EXPECT_EQ(sizeof(ExpectedExportedSymbols) / sizeof(ExportedSymbol),
diff --git a/llvm/unittests/TextAPI/TextStubV5Tests.cpp b/llvm/unittests/TextAPI/TextStubV5Tests.cpp
@@ -877,7 +877,7 @@ TEST(TBDv5, WriteMultipleDocuments) {
   NestedFileB.setCurrentVersion(PackedVersion(1, 0, 0));
   NestedFileB.setTwoLevelNamespace();
   NestedFileB.setApplicationExtensionSafe(true);
-  NestedFileB.addSymbol(SymbolKind::GlobalSymbol, "_varFooBaz", AllTargets,
+  NestedFileB.addSymbol(SymbolKind::GlobalSymbol, "_varFooBaz", {AllTargets[0]},
                         SymbolFlags::Data);
   File.addDocument(std::make_shared<InterfaceFile>(std::move(NestedFileB)));
 
