[TextAPI] Add support for TBDv5 Files to nm & tapi-diff

[cishida]

Summary

This includes handling of new attributes for symbols & rpath.
In the event that an older format file is compared to tbd_v5, ignore these new attributes.

[cishida]

Handle multiple flags set in llvm-tapi-diff printing

[jhenderson]

I got pinged on this due to the llvm-nm change, so I took a quick look through and spotted a few nits/test issues. Someone who is more knowledgeable about TAPI/TBD etc stuff should properly review though.

llvm/lib/TextAPI/Symbol.cpp

58–59

llvm/test/Object/nm-tapi.test

71

Aside: It was confusing that the llvm-nm related testing is not in the test/tools/llvm-nm folder, unlike most llvm-nm testing. Perhaps worth considering moving this test in a separate change?

llvm/test/tools/llvm-tapi-diff/v5.test

4–5

Presumably you need to add 2>&1 here to capture stderr for any warnings?

6

Nit: duplicate blank line.

8

Isn't this unnecessary? Won't llvm-tapi-diff fail with a non-zero exit code if it encounters and error?

24

Nit: looks like you've got an extra trailing blank line.

[cishida]

Fix up patch based on review comments

[cishida]

llvm/test/Object/nm-tapi.test

71

Yes, that's fair. Thinking back, the rationale was testing the libObject changes indirectly but I can certainly move this to test/tools/llvm-nm in a followup patch to avoid confusion.

llvm/test/tools/llvm-tapi-diff/v5.test

8

llvm-tapi-diff currently doesn't emit any warnings so all errors are fatal. In these two invocations the tool shouldn't report any differences.

[ributzka]

LGTM

llvm/include/llvm/Object/TapiFile.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJECT_TAPIFILE_H
 #define LLVM_OBJECT_TAPIFILE_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TextAPI/Architecture.h"
 
 namespace llvm {
 
 class raw_ostream;
 
 namespace MachO {
 
 class InterfaceFile;
 
 }
 
 namespace object {
 
 class TapiFile : public SymbolicFile {
 public:
-  TapiFile(MemoryBufferRef Source, const MachO::InterfaceFile &interface,
+  TapiFile(MemoryBufferRef Source, const MachO::InterfaceFile &Interface,
            MachO::Architecture Arch);
   ~TapiFile() override;
 
   void moveSymbolNext(DataRefImpl &DRI) const override;
 
   Error printSymbolName(raw_ostream &OS, DataRefImpl DRI) const override;
 
   Expected<uint32_t> getSymbolFlags(DataRefImpl DRI) const override;
 
   basic_symbol_iterator symbol_begin() const override;
 
   basic_symbol_iterator symbol_end() const override;
 
+  Expected<SymbolRef::Type> getSymbolType(DataRefImpl DRI) const;
+
   static bool classof(const Binary *v) { return v->isTapiFile(); }
 
   bool is64Bit() const override { return MachO::is64Bit(Arch); }
 
 private:
   struct Symbol {
     StringRef Prefix;
     StringRef Name;
     uint32_t Flags;
+    SymbolRef::Type Type;
 
-    constexpr Symbol(StringRef Prefix, StringRef Name, uint32_t Flags)
-        : Prefix(Prefix), Name(Name), Flags(Flags) {}
+    constexpr Symbol(StringRef Prefix, StringRef Name, uint32_t Flags,
+                     SymbolRef::Type Type)
+        : Prefix(Prefix), Name(Name), Flags(Flags), Type(Type) {}
   };
 
   std::vector<Symbol> Symbols;
   MachO::Architecture Arch;
 };
 
 } // end namespace object.
 } // end namespace llvm.
 
 #endif // LLVM_OBJECT_TAPIFILE_H

llvm/include/llvm/TextAPI/Symbol.h

using const_filtered_target_range =
       llvm::iterator_range<const_filtered_target_iterator>;
   const_filtered_target_range targets(ArchitectureSet architectures) const;
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   void dump(raw_ostream &OS) const;
   void dump() const { dump(llvm::errs()); }
 #endif
 
-  bool operator==(const Symbol &O) const {
-    return std::tie(Name, Kind, Targets, Flags) ==
-           std::tie(O.Name, O.Kind, O.Targets, O.Flags);
-  }
+  bool operator==(const Symbol &O) const;
 
   bool operator!=(const Symbol &O) const { return !(*this == O); }
 
   bool operator<(const Symbol &O) const {
     return std::tie(Name, Kind, Targets, Flags) <
            std::tie(O.Name, O.Kind, O.Targets, O.Flags);
   }
 

llvm/lib/BinaryFormat/Magic.cpp

if (startswith(Magic, "MDMP"))
       return file_magic::minidump;
     break;
 
   case 0x64: // x86-64 or ARM64 Windows.
     if (Magic[1] == char(0x86) || Magic[1] == char(0xaa))
       return file_magic::coff_object;
     break;
 
-  case 0x2d: // YAML '-'
+  case 0x2d: // YAML '-' MachO TBD.
     if (startswith(Magic, "--- !tapi") || startswith(Magic, "---\narchs:"))
       return file_magic::tapi_file;
     break;
-  
+  case 0x7b: // JSON '{' MachO TBD.
+    return file_magic::tapi_file;
+    break;
   case 'D': // DirectX container file - DXBC
     if (startswith(Magic, "DXBC"))
       return file_magic::dxcontainer_object;
     break;
 
   case 0x41: // ARM64EC windows
     if (Magic[1] == char(0xA6))
       return file_magic::coff_object;

llvm/lib/Object/TapiFile.cpp

else
     Flags |= BasicSymbolRef::SF_Exported;
 
   if (Sym->isWeakDefined() || Sym->isWeakReferenced())
     Flags |= BasicSymbolRef::SF_Weak;
 
   return Flags;
 }
 
-TapiFile::TapiFile(MemoryBufferRef Source, const InterfaceFile &interface,
+static SymbolRef::Type getType(const Symbol *Sym) {
+  SymbolRef::Type Type = SymbolRef::ST_Unknown;
+  if (Sym->isData())
+    Type = SymbolRef::ST_Data;
+  else if (Sym->isText())
+    Type = SymbolRef::ST_Function;
+
+  return Type;
+}
+
+TapiFile::TapiFile(MemoryBufferRef Source, const InterfaceFile &Interface,
                    Architecture Arch)
     : SymbolicFile(ID_TapiFile, Source), Arch(Arch) {
-  for (const auto *Symbol : interface.symbols()) {
+  for (const auto *Symbol : Interface.symbols()) {
     if (!Symbol->getArchitectures().has(Arch))
       continue;
 
     switch (Symbol->getKind()) {
     case SymbolKind::GlobalSymbol:
-      Symbols.emplace_back(StringRef(), Symbol->getName(), getFlags(Symbol));
+      Symbols.emplace_back(StringRef(), Symbol->getName(), getFlags(Symbol),
+                           ::getType(Symbol));
       break;
     case SymbolKind::ObjectiveCClass:
-      if (interface.getPlatforms().count(PLATFORM_MACOS) && Arch == AK_i386) {
+      if (Interface.getPlatforms().count(PLATFORM_MACOS) && Arch == AK_i386) {
         Symbols.emplace_back(ObjC1ClassNamePrefix, Symbol->getName(),
-                             getFlags(Symbol));
+                             getFlags(Symbol), ::getType(Symbol));
       } else {
         Symbols.emplace_back(ObjC2ClassNamePrefix, Symbol->getName(),
-                             getFlags(Symbol));
+                             getFlags(Symbol), ::getType(Symbol));
         Symbols.emplace_back(ObjC2MetaClassNamePrefix, Symbol->getName(),
-                             getFlags(Symbol));
+                             getFlags(Symbol), ::getType(Symbol));
       }
       break;
     case SymbolKind::ObjectiveCClassEHType:
       Symbols.emplace_back(ObjC2EHTypePrefix, Symbol->getName(),
-                           getFlags(Symbol));
+                           getFlags(Symbol), ::getType(Symbol));
       break;
     case SymbolKind::ObjectiveCInstanceVariable:
-      Symbols.emplace_back(ObjC2IVarPrefix, Symbol->getName(),
-                           getFlags(Symbol));
+      Symbols.emplace_back(ObjC2IVarPrefix, Symbol->getName(), getFlags(Symbol),
+                           ::getType(Symbol));
       break;
     }
   }
 }
 
 TapiFile::~TapiFile() = default;
 
 void TapiFile::moveSymbolNext(DataRefImpl &DRI) const { DRI.d.a++; }
 
 Error TapiFile::printSymbolName(raw_ostream &OS, DataRefImpl DRI) const {
   assert(DRI.d.a < Symbols.size() && "Attempt to access symbol out of bounds");
   const Symbol &Sym = Symbols[DRI.d.a];
   OS << Sym.Prefix << Sym.Name;
   return Error::success();
 }
 
+Expected<SymbolRef::Type> TapiFile::getSymbolType(DataRefImpl DRI) const {
+  assert(DRI.d.a < Symbols.size() && "Attempt to access symbol out of bounds");
+  return Symbols[DRI.d.a].Type;
+}
+
 Expected<uint32_t> TapiFile::getSymbolFlags(DataRefImpl DRI) const {
   assert(DRI.d.a < Symbols.size() && "Attempt to access symbol out of bounds");
   return Symbols[DRI.d.a].Flags;
 }
 
 basic_symbol_iterator TapiFile::symbol_begin() const {
   DataRefImpl DRI;
   DRI.d.a = 0;
   return BasicSymbolRef{DRI, this};
 }
 
 basic_symbol_iterator TapiFile::symbol_end() const {
   DataRefImpl DRI;
   DRI.d.a = Symbols.size();
   return BasicSymbolRef{DRI, this};
 }

llvm/lib/TextAPI/InterfaceFile.cpp

auto Pos = llvm::lower_bound(Documents, Document,
                                [](const std::shared_ptr<InterfaceFile> &LHS,
                                   const std::shared_ptr<InterfaceFile> &RHS) {
                                  return LHS->InstallName < RHS->InstallName;
                                });
   Document->Parent = this;
   Documents.insert(Pos, Document);
 }
 
+static bool isYAMLTextStub(const FileType &Kind) {
+  return (Kind >= FileType::TBD_V1) && (Kind < FileType::TBD_V5);
+}
+
 bool InterfaceFile::operator==(const InterfaceFile &O) const {
   if (Targets != O.Targets)
     return false;
   if (InstallName != O.InstallName)
     return false;
   if ((CurrentVersion != O.CurrentVersion) ||
       (CompatibilityVersion != O.CompatibilityVersion))
     return false;
   if (SwiftABIVersion != O.SwiftABIVersion)
     return false;
   if (IsTwoLevelNamespace != O.IsTwoLevelNamespace)
     return false;
   if (IsAppExtensionSafe != O.IsAppExtensionSafe)
     return false;
   if (IsInstallAPI != O.IsInstallAPI)
     return false;
   if (ParentUmbrellas != O.ParentUmbrellas)
     return false;
   if (AllowableClients != O.AllowableClients)
     return false;
   if (ReexportedLibraries != O.ReexportedLibraries)
     return false;
   if (Symbols != O.Symbols)
     return false;
+  // Don't compare run search paths for older filetypes that cannot express
+  // them.
+  if (!(isYAMLTextStub(FileKind)) && !(isYAMLTextStub(O.FileKind))) {
+    if (RPaths != O.RPaths)
+      return false;
+  }
+
   if (!std::equal(Documents.begin(), Documents.end(), O.Documents.begin(),
                   O.Documents.end(),
                   [](const std::shared_ptr<InterfaceFile> LHS,
                      const std::shared_ptr<InterfaceFile> RHS) {
                     return *LHS == *RHS;
                   }))
     return false;
   return true;
 }

llvm/lib/TextAPI/Symbol.cpp

 Symbol::targets(ArchitectureSet Architectures) const {
   std::function<bool(const Target &)> FN =
       [Architectures](const Target &Target) {
         return Architectures.has(Target.Arch);
       };
   return make_filter_range(Targets, FN);
 }
 
+bool Symbol::operator==(const Symbol &O) const {
+  // Older Tapi files do not express all these symbol flags. In those
+  // cases, ignore those differences.
+  auto RemoveFlag = [](const Symbol &Sym, SymbolFlags &Flag) {
+    if (Sym.isData())
+      Flag &= ~SymbolFlags::Data;
+    if (Sym.isText())
+      Flag &= ~SymbolFlags::Text;
+  };
+  SymbolFlags LHSFlags = Flags;
+  SymbolFlags RHSFlags = O.Flags;
+  if ((!O.isData() && !O.isText()) || (!isData() && !isText())) {
+    RemoveFlag(*this, LHSFlags);
+    RemoveFlag(O, RHSFlags);
+  }
+  return std::tie(Name, Kind, Targets, LHSFlags) ==
+         std::tie(O.Name, O.Kind, O.Targets, RHSFlags);
+}
+
 } // end namespace MachO.
 } // end namespace llvm.

llvm/test/Object/Inputs/tapi-v5.tbd

+{
+    "main_library": {
+        "current_versions": [
+            {
+                "version": "1.2.3"
+            }
+        ],
+        "exported_symbols": [
+            {
+                "data": {
+                    "global": [
+                        "_publicGlobalVariable",
+                        "_extraGlobalAPI1",
+                        "_privateGlobalVariable"
+                    ],
+                    "objc_class": [
+                        "SubClass",
+                        "Basic6",
+                        "Basic1",
+                        "Base",
+                        "Basic3",
+                        "FooClass",
+                        "Basic4",
+                        "ExternalManagedObject"
+                    ],
+                    "objc_eh_type": [
+                        "SubClass",
+                        "Base"
+                    ],
+                    "objc_ivar": [
+                        "Basic4.ivar2",
+                        "Basic4.ivar1",
+                        "Basic4_2.ivar2"
+                    ],
+                    "weak": [
+                        "_weakPrivateGlobalVariable",
+                        "_weakPublicGlobalVariable"
+                    ]
+                }
+            },
+            {
+                "text": {
+                    "global": [
+                        "_publicGlobalFunc"
+                    ]
+                }
+            }
+        ],
+        "flags": [
+            {
+                "attributes": [
+                    "not_app_extension_safe"
+                ]
+            }
+        ],
+        "install_names": [
+            {
+                "name": "/System/Library/Frameworks/Simple.framework/Versions/A/Simple"
+            }
+        ],
+        "target_info": [
+            {
+                "min_deployment": "10.10",
+                "target": "x86_64-macos"
+            },
+            {
+                "min_deployment": "10.10",
+                "target": "arm64-macos"
+            }
+        ]
+    },
+    "tapi_tbd_version": 5
+}

llvm/test/Object/nm-tapi.test

 RUN: llvm-nm %p/Inputs/tapi-v1.tbd 2>&1\
 RUN:          | FileCheck %s -check-prefix V1
 
 RUN: llvm-nm %p/Inputs/tapi-v2.tbd \
 RUN:          | FileCheck %s -check-prefix V2
 
 RUN: llvm-nm --add-inlinedinfo --arch=x86_64 %p/Inputs/tapi-v3.tbd \
 RUN:          | FileCheck %s -check-prefix V3
 
 RUN: llvm-nm %p/Inputs/tapi-v4.tbd \
 RUN:          | FileCheck %s -check-prefix V4
 
 RUN: llvm-nm %p/Inputs/tapi-v4-watchos.tbd \
 RUN:          | FileCheck %s -check-prefix V4-WATCH
 
+RUN: llvm-nm %p/Inputs/tapi-v5.tbd \
+RUN:          | FileCheck %s -check-prefix V5
+
 V1: /u/l/libfoo.dylib (for architecture armv7):
 V1-NEXT: 00000000 S _sym
 V1: /u/l/libfoo.dylib (for architecture armv7s):
 V1-NEXT: 00000000 S _sym
 
 V2: /u/l/libfoo.dylib (for architecture armv7):
 V2-NEXT: 00000000 S _sym1
 V2-NEXT: 00000000 S _sym2

 V4: /u/l/libFoo.dylib (for architecture x86_64):
 V4-NEXT: 0000000000000000 S _sym3
 V4-NEXT: 0000000000000000 S _sym4
 
 V4-WATCH: /u/l/libFoo.dylib (for architecture armv7k)
 V4-WATCH-NEXT: 00000000 S _sym1
 V4-WATCH: /u/l/libFoo.dylib (for architecture arm64_32)
 V4-WATCH-NEXT: 00000000 S _sym1
+
+V5: /System/Library/Frameworks/Simple.framework/Versions/A/Simple (for architecture x86_64):
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic1
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic3
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic4
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic6
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_ExternalManagedObject
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_FooClass
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_SubClass
+V5-NEXT: 0000000000000000 D _OBJC_EHTYPE_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_EHTYPE_$_SubClass
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4.ivar1
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4.ivar2
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4_2.ivar2
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic1
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic3
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic4
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic6
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_ExternalManagedObject
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_FooClass
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_SubClass
+V5-NEXT: 0000000000000000 D _extraGlobalAPI1
+V5-NEXT: 0000000000000000 D _privateGlobalVariable
+V5-NEXT: 0000000000000000 T _publicGlobalFunc
+V5-NEXT: 0000000000000000 D _publicGlobalVariable
+V5-NEXT: 0000000000000000 W _weakPrivateGlobalVariable
+V5-NEXT: 0000000000000000 W _weakPublicGlobalVariable
+
+V5: /System/Library/Frameworks/Simple.framework/Versions/A/Simple (for architecture arm64):
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic1
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic3
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic4
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Basic6
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_ExternalManagedObject
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_FooClass
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_SubClass
+V5-NEXT: 0000000000000000 D _OBJC_EHTYPE_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_EHTYPE_$_SubClass
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4.ivar1
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4.ivar2
+V5-NEXT: 0000000000000000 D _OBJC_IVAR_$_Basic4_2.ivar2
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Base
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic1
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic3
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic4
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_Basic6
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_ExternalManagedObject
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_FooClass
+V5-NEXT: 0000000000000000 D _OBJC_METACLASS_$_SubClass
+V5-NEXT: 0000000000000000 D _extraGlobalAPI1
+V5-NEXT: 0000000000000000 D _privateGlobalVariable
+V5-NEXT: 0000000000000000 T _publicGlobalFunc
+V5-NEXT: 0000000000000000 D _publicGlobalVariable
+V5-NEXT: 0000000000000000 W _weakPrivateGlobalVariable
+V5-NEXT: 0000000000000000 W _weakPublicGlobalVariable

llvm/test/tools/llvm-tapi-diff/v5.test

+; RUN: rm -rf %t
+; RUN: split-file %s %t  
+; RUN: llvm-tapi-diff %t/Simple_v5.tbd  %t/Simple_v5.tbd 2>&1 | FileCheck %s --allow-empty
+; RUN: llvm-tapi-diff %t/Simple_v5.tbd  %t/Simple_v4.tbd 2>&1 | FileCheck %s --allow-empty
+
+; CHECK-NOT: error:
+; CHECK-NOT: warning:
+
+//--- Simple_v4.tbd
+--- !tapi-tbd
+tbd-version:     4
+targets:         [ x86_64-macos, arm64-macos ]
+flags:           [ not_app_extension_safe ]
+install-name:    '@rpath/libFake.dylib'
+exports:
+  - targets:         [ x86_64-macos, arm64-macos ]
+    symbols:         [ _foo ]
+...
+
+//--- Simple_v5.tbd
+{"main_library":{"exported_symbols":[{"text":{"global":["_foo"]}}],"flags":[{"attributes":["not_app_extension_safe"]}],"install_names":[{"name":"@rpath/libFake.dylib"}],"target_info":[{"min_deployment":"13","target":"x86_64-macos"},{"min_deployment":"13","target":"arm64-macos"}]},"tapi_tbd_version":5}

llvm/tools/llvm-nm/llvm-nm.cpp

case MachO::N_SECT: {
     return 's';
   }
   }
 
   return '?';
 }
 
 static char getSymbolNMTypeChar(TapiFile &Obj, basic_symbol_iterator I) {
+  auto Type = cantFail(Obj.getSymbolType(I->getRawDataRefImpl()));
+  switch (Type) {
+  case SymbolRef::ST_Data:
+    return 'd';
+  case SymbolRef::ST_Function:
+    return 't';
+  default:
     return 's';
   }
+}
 
 static char getSymbolNMTypeChar(WasmObjectFile &Obj, basic_symbol_iterator I) {
   uint32_t Flags = cantFail(I->getFlags());
   if (Flags & SymbolRef::SF_Executable)
     return 't';
   return 'd';
 }
 

llvm/tools/llvm-tapi-diff/DiffEngine.h

 };
 
 /// SymScalar is the diff symbol and the order.
 class SymScalar {
 public:
   SymScalar(InterfaceInputOrder Order, const MachO::Symbol *Sym)
       : Order(Order), Val(Sym){};
 
-  std::string getFlagString(MachO::SymbolFlags Flags) {
-    return Flags != MachO::SymbolFlags::None
-               ? " - " + stringifySymbolFlag(Flags)
-               : stringifySymbolFlag(Flags);
-  }
+  std::string getFlagString(const MachO::Symbol *Sym);
 
   void print(raw_ostream &OS, std::string Indent, MachO::Target Targ);
 
   const MachO::Symbol *getVal() const { return Val; }
   InterfaceInputOrder getOrder() const { return Order; }
 
 private:
   /// The order is the file from which the diff is found.
   InterfaceInputOrder Order;
   const MachO::Symbol *Val;
   StringLiteral getSymbolNamePrefix(MachO::SymbolKind Kind);
-  std::string stringifySymbolFlag(MachO::SymbolFlags Flag);
 };
 
 class DiffStrVec : public AttributeDiff {
 public:
   MachO::Target Targ;
   /// Values is a vector of StringRef values associated with the target.
   std::vector<DiffScalarVal<StringRef, AD_Diff_Scalar_Str>> TargValues;
   DiffStrVec(MachO::Target Targ) : AttributeDiff(AD_Str_Vec), Targ(Targ){};

llvm/tools/llvm-tapi-diff/DiffEngine.cpp

StringLiteral SymScalar::getSymbolNamePrefix(MachO::SymbolKind Kind) {
   case MachO::SymbolKind ::ObjectiveCClassEHType:
     return ObjC2EHTypePrefix;
   case MachO::SymbolKind ::ObjectiveCInstanceVariable:
     return ObjC2IVarPrefix;
   }
   llvm_unreachable("Unknown llvm::MachO::SymbolKind enum");
 }
 
-std::string SymScalar::stringifySymbolFlag(MachO::SymbolFlags Flag) {
-  switch (Flag) {
-  case MachO::SymbolFlags::None:
-    return "";
-  case MachO::SymbolFlags::ThreadLocalValue:
-    return "Thread-Local";
-  case MachO::SymbolFlags::WeakDefined:
-    return "Weak-Defined";
-  case MachO::SymbolFlags::WeakReferenced:
-    return "Weak-Referenced";
-  case MachO::SymbolFlags::Undefined:
-    return "Undefined";
-  case MachO::SymbolFlags::Rexported:
-    return "Reexported";
-  default:
-    return "";
-  }
-  llvm_unreachable("Unknown llvm::MachO::SymbolFlags enum");
+std::string SymScalar::getFlagString(const MachO::Symbol *Sym) {
+  if (Sym->getFlags() == SymbolFlags::None)
+    return {};
+  SmallString<64> Flags(" - ");
+  if (Sym->isThreadLocalValue())
+    Flags.append("Thread-Local ");
+  if (Sym->isWeakDefined())
+    Flags.append("Weak-Defined ");
+  if (Sym->isWeakReferenced())
+    Flags.append("Weak-Referenced ");
+  if (Sym->isUndefined())
+    Flags.append("Undefined ");
+  if (Sym->isReexported())
+    Flags.append("Reexported ");
+  if (Sym->isData())
+    Flags.append("Data ");
+  if (Sym->isText())
+    Flags.append("Text ");
+
+  return std::string(Flags);
 }
 
 void SymScalar::print(raw_ostream &OS, std::string Indent, MachO::Target Targ) {
   if (Val->getKind() == MachO::SymbolKind::ObjectiveCClass) {
     if (Targ.Arch == MachO::AK_i386 && Targ.Platform == MachO::PLATFORM_MACOS) {
       OS << Indent << "\t\t" << ((Order == lhs) ? "< " : "> ")
-         << ObjC1ClassNamePrefix << Val->getName()
-         << getFlagString(Val->getFlags()) << "\n";
+         << ObjC1ClassNamePrefix << Val->getName() << getFlagString(Val)
+         << "\n";
       return;
     }
     OS << Indent << "\t\t" << ((Order == lhs) ? "< " : "> ")
-       << ObjC2ClassNamePrefix << Val->getName()
-       << getFlagString(Val->getFlags()) << "\n";
+       << ObjC2ClassNamePrefix << Val->getName() << getFlagString(Val) << "\n";
   }
   OS << Indent << "\t\t" << ((Order == lhs) ? "< " : "> ")
      << getSymbolNamePrefix(Val->getKind()) << Val->getName()
-     << getFlagString(Val->getFlags()) << "\n";
+     << getFlagString(Val) << "\n";
 }
 
 bool checkSymbolEquality(llvm::MachO::InterfaceFile::const_symbol_range LHS,
                          llvm::MachO::InterfaceFile::const_symbol_range RHS) {
   return std::equal(LHS.begin(), LHS.end(), RHS.begin(),
                     [&](auto LHS, auto RHS) { return *LHS == *RHS; });
 }