[TBDGen] Return a vector of symbols instead of set

A couple of clients are iterating over the result,
so switch to a vector to ensure we don't
accidentally introduce any non-determinism.

Author: hamishknight

include/swift/AST/IRGenRequests.h

@@ -184,7 +184,7 @@ struct IRGenDescriptor {
   ModuleDecl *getParentModule() const;
 
   /// Compute the linker directives to emit.
-  llvm::StringSet<> getLinkerDirectives() const;
+  std::vector<std::string> getLinkerDirectives() const;
 };
 
 /// Report that a request of the given kind is being evaluated, so it

include/swift/AST/TBDGenRequests.h

@@ -78,7 +78,7 @@ void simple_display(llvm::raw_ostream &out, const TBDGenDescriptor &desc);
 SourceLoc extractNearestSourceLoc(const TBDGenDescriptor &desc);
 
 using TBDFileAndSymbols =
-    std::pair<llvm::MachO::InterfaceFile, llvm::StringSet<>>;
+    std::pair<llvm::MachO::InterfaceFile, std::vector<std::string>>;
 
 /// Computes the TBD file and public symbols for a given module or file.
 class GenerateTBDRequest

include/swift/TBDGen/TBDGen.h

@@ -89,7 +89,7 @@ struct TBDGenOptions {
   }
 };
 
-llvm::StringSet<> getPublicSymbols(TBDGenDescriptor desc);
+std::vector<std::string> getPublicSymbols(TBDGenDescriptor desc);
 
 void writeTBDFile(ModuleDecl *M, llvm::raw_ostream &os,
                   const TBDGenOptions &opts);

lib/FrontendTool/FrontendTool.cpp

@@ -1074,7 +1074,7 @@ static bool writeLdAddCFileIfNeeded(CompilerInstance &Instance) {
     llvm::raw_svector_ostream NameOS(NameBuffer);
     NameOS << "ldAdd_" << Idx;
     OS << "extern const char " << NameOS.str() << " __asm(\"" <<
-      changeToLdAdd(S.getKey()) << "\");\n";
+      changeToLdAdd(S) << "\");\n";
     OS << "const char " << NameOS.str() << " = 0;\n";
     ++ Idx;
   }

lib/FrontendTool/TBD.cpp

@@ -73,10 +73,13 @@ bool swift::inputFileKindCanHaveTBDValidated(InputFileKind kind) {
   llvm_unreachable("unhandled kind");
 }
 
-static bool validateSymbolSet(DiagnosticEngine &diags,
-                              llvm::StringSet<> symbols,
-                              const llvm::Module &IRModule,
-                              bool diagnoseExtraSymbolsInTBD) {
+static bool validateSymbols(DiagnosticEngine &diags,
+                            const std::vector<std::string> &symbols,
+                            const llvm::Module &IRModule,
+                            bool diagnoseExtraSymbolsInTBD) {
+  llvm::StringSet<> symbolSet;
+  symbolSet.insert(symbols.begin(), symbols.end());
+
   auto error = false;
 
   // Diff the two sets of symbols, flagging anything outside their intersection.
@@ -101,13 +104,13 @@ static bool validateSymbolSet(DiagnosticEngine &diags,
           GV->hasExternalLinkage() && !GV->hasHiddenVisibility();
       if (!GV->isDeclaration() && externallyVisible) {
         // Is it listed?
-        if (!symbols.erase(name))
+        if (!symbolSet.erase(name))
           // Note: Add the unmangled name to the irNotTBD list, which is owned
           //       by the IRModule, instead of the mangled name.
           irNotTBD.push_back(unmangledName);
       }
     } else {
-      assert(symbols.find(name) == symbols.end() &&
+      assert(symbolSet.find(name) == symbolSet.end() &&
              "non-global value in value symbol table");
     }
   }
@@ -121,7 +124,7 @@ static bool validateSymbolSet(DiagnosticEngine &diags,
 
   if (diagnoseExtraSymbolsInTBD) {
     // Look for any extra symbols.
-    for (auto &name : sortSymbols(symbols)) {
+    for (auto &name : sortSymbols(symbolSet)) {
       diags.diagnose(SourceLoc(), diag::symbol_in_tbd_not_in_ir, name,
                      Demangle::demangleSymbolAsString(name));
       error = true;
@@ -140,16 +143,15 @@ bool swift::validateTBD(ModuleDecl *M,
                         const TBDGenOptions &opts,
                         bool diagnoseExtraSymbolsInTBD) {
   auto symbols = getPublicSymbols(TBDGenDescriptor::forModule(M, opts));
-  return validateSymbolSet(M->getASTContext().Diags, symbols, IRModule,
-                           diagnoseExtraSymbolsInTBD);
+  return validateSymbols(M->getASTContext().Diags, symbols, IRModule,
+                         diagnoseExtraSymbolsInTBD);
 }
 
 bool swift::validateTBD(FileUnit *file,
                         const llvm::Module &IRModule,
                         const TBDGenOptions &opts,
                         bool diagnoseExtraSymbolsInTBD) {
   auto symbols = getPublicSymbols(TBDGenDescriptor::forFile(file, opts));
-  return validateSymbolSet(file->getParentModule()->getASTContext().Diags,
-                           symbols, IRModule,
-                           diagnoseExtraSymbolsInTBD);
+  return validateSymbols(file->getParentModule()->getASTContext().Diags,
+                         symbols, IRModule, diagnoseExtraSymbolsInTBD);
 }

lib/IRGen/GenDecl.cpp

@@ -1031,7 +1031,7 @@ static bool hasCodeCoverageInstrumentation(SILFunction &f, SILModule &m) {
 }
 
 void IRGenerator::emitGlobalTopLevel(
-    const llvm::StringSet<> &linkerDirectives) {
+    const std::vector<std::string> &linkerDirectives) {
   // Generate order numbers for the functions in the SIL module that
   // correspond to definitions in the LLVM module.
   unsigned nextOrderNumber = 0;
@@ -1051,8 +1051,8 @@ void IRGenerator::emitGlobalTopLevel(
     CurrentIGMPtr IGM = getGenModule(wt.getProtocol()->getDeclContext());
     ensureRelativeSymbolCollocation(wt);
   }
-  for (auto &entry: linkerDirectives) {
-    createLinkerDirectiveVariable(*PrimaryIGM, entry.getKey());
+  for (auto &directive: linkerDirectives) {
+    createLinkerDirectiveVariable(*PrimaryIGM, directive);
   }
   for (SILGlobalVariable &v : PrimaryIGM->getSILModule().getSILGlobals()) {
     Decl *decl = v.getDecl();

lib/IRGen/IRGenModule.h

@@ -375,7 +375,7 @@ class IRGenerator {
 
   /// Emit functions, variables and tables which are needed anyway, e.g. because
   /// they are externally visible.
-  void emitGlobalTopLevel(const llvm::StringSet<> &LinkerDirectives);
+  void emitGlobalTopLevel(const std::vector<std::string> &LinkerDirectives);
 
   /// Emit references to each of the protocol descriptors defined in this
   /// IR module.

lib/IRGen/IRGenRequests.cpp

@@ -76,7 +76,7 @@ ModuleDecl *IRGenDescriptor::getParentModule() const {
   return Ctx.get<ModuleDecl *>();
 }
 
-llvm::StringSet<> IRGenDescriptor::getLinkerDirectives() const {
+std::vector<std::string> IRGenDescriptor::getLinkerDirectives() const {
   auto opts = TBDOpts;
   opts.LinkerDirectivesOnly = true;
   if (auto *SF = Ctx.dyn_cast<SourceFile *>()) {

lib/TBDGen/TBDGen.cpp

@@ -67,11 +67,10 @@ void TBDGenVisitor::addSymbolInternal(StringRef name,
   if (!isLinkerDirective && Opts.LinkerDirectivesOnly)
     return;
   Symbols.addSymbol(kind, name, Targets);
-  if (StringSymbols && kind == SymbolKind::GlobalSymbol) {
-    auto isNewValue = StringSymbols->insert(name).second;
-    (void)isNewValue;
+  if (kind == SymbolKind::GlobalSymbol) {
+    StringSymbols.push_back(name);
 #ifndef NDEBUG
-    if (!isNewValue) {
+    if (!DuplicateSymbolChecker.insert(name).second) {
       llvm::dbgs() << "TBDGen duplicate symbol: " << name << '\n';
       assert(false && "TBDGen symbol appears twice");
     }
@@ -1135,10 +1134,8 @@ GenerateTBDRequest::evaluate(Evaluator &evaluator,
     llvm::MachO::Target targetVar(*ctx.LangOpts.TargetVariant);
     file.addTarget(targetVar);
   }
-  StringSet symbols;
   auto *clang = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
-  TBDGenVisitor visitor(file, {target}, &symbols,
-                        clang->getTargetInfo().getDataLayout(),
+  TBDGenVisitor visitor(file, {target}, clang->getTargetInfo().getDataLayout(),
                         linkInfo, M, opts);
 
   auto visitFile = [&](FileUnit *file) {
@@ -1187,10 +1184,10 @@ GenerateTBDRequest::evaluate(Evaluator &evaluator,
     });
   }
 
-  return std::make_pair(std::move(file), std::move(symbols));
+  return std::make_pair(std::move(file), std::move(visitor.StringSymbols));
 }
 
-StringSet swift::getPublicSymbols(TBDGenDescriptor desc) {
+std::vector<std::string> swift::getPublicSymbols(TBDGenDescriptor desc) {
   auto &evaluator = desc.getParentModule()->getASTContext().evaluator;
   return llvm::cantFail(evaluator(GenerateTBDRequest{desc})).second;
 }

lib/TBDGen/TBDGenVisitor.h

@@ -63,9 +63,14 @@ class TBDGenVisitor : public ASTVisitor<TBDGenVisitor> {
 public:
   llvm::MachO::InterfaceFile &Symbols;
   llvm::MachO::TargetList Targets;
-  StringSet *StringSymbols;
+  std::vector<std::string> StringSymbols;
   const llvm::DataLayout &DataLayout;
 
+#ifndef NDEBUG
+  /// Tracks the symbols emitted to ensure we don't emit any duplicates.
+  llvm::StringSet<> DuplicateSymbolChecker;
+#endif
+
   const UniversalLinkageInfo &UniversalLinkInfo;
   ModuleDecl *SwiftModule;
   const TBDGenOptions &Opts;
@@ -136,13 +141,13 @@ class TBDGenVisitor : public ASTVisitor<TBDGenVisitor> {
 
 public:
   TBDGenVisitor(llvm::MachO::InterfaceFile &symbols,
-                llvm::MachO::TargetList targets, StringSet *stringSymbols,
+                llvm::MachO::TargetList targets,
                 const llvm::DataLayout &dataLayout,
                 const UniversalLinkageInfo &universalLinkInfo,
                 ModuleDecl *swiftModule, const TBDGenOptions &opts)
-      : Symbols(symbols), Targets(targets), StringSymbols(stringSymbols),
-        DataLayout(dataLayout), UniversalLinkInfo(universalLinkInfo),
-        SwiftModule(swiftModule), Opts(opts),
+      : Symbols(symbols), Targets(targets), DataLayout(dataLayout),
+        UniversalLinkInfo(universalLinkInfo), SwiftModule(swiftModule),
+        Opts(opts),
         previousInstallNameMap(parsePreviousModuleInstallNameMap())  {}
   ~TBDGenVisitor() { assert(DeclStack.empty()); }
   void addMainIfNecessary(FileUnit *file) {