[ORC] Simplify VSO::lookupFlags to return the flags map.

This discards the unresolved symbols set and returns the flags map directly
(rather than mutating it via the first argument).

The unresolved symbols result made it easy to chain lookupFlags calls, but such
chaining should be rare to non-existant (especially now that symbol resolvers
are being deprecated) so the simpler method signature is preferable.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@337594 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/ExecutionEngine/Orc/CompileOnDemandLayer.h

@@ -499,20 +499,29 @@ private:
     };
 
     auto GVsResolver = createSymbolResolver(
-        [&LD, LegacyLookup](SymbolFlagsMap &SymbolFlags,
-                            const SymbolNameSet &Symbols) {
-          auto NotFoundViaLegacyLookup =
-              lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
+        [&LD, LegacyLookup](const SymbolNameSet &Symbols) {
+          auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
 
-          if (!NotFoundViaLegacyLookup) {
-            logAllUnhandledErrors(NotFoundViaLegacyLookup.takeError(), errs(),
+          if (!SymbolFlags) {
+            logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
                                   "CODLayer/GVsResolver flags lookup failed: ");
-            SymbolFlags.clear();
-            return SymbolNameSet();
+            return SymbolFlagsMap();
           }
 
-          return LD.BackingResolver->lookupFlags(SymbolFlags,
-                                                 *NotFoundViaLegacyLookup);
+          if (SymbolFlags->size() == Symbols.size())
+            return *SymbolFlags;
+
+          SymbolNameSet NotFoundViaLegacyLookup;
+          for (auto &S : Symbols)
+            if (!SymbolFlags->count(S))
+              NotFoundViaLegacyLookup.insert(S);
+          auto SymbolFlags2 =
+              LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+
+          for (auto &KV : SymbolFlags2)
+            (*SymbolFlags)[KV.first] = std::move(KV.second);
+
+          return *SymbolFlags;
         },
         [this, &LD,
          LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -659,18 +668,29 @@ private:
 
     // Create memory manager and symbol resolver.
     auto Resolver = createSymbolResolver(
-        [&LD, LegacyLookup](SymbolFlagsMap &SymbolFlags,
-                            const SymbolNameSet &Symbols) {
-          auto NotFoundViaLegacyLookup =
-              lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
-          if (!NotFoundViaLegacyLookup) {
-            logAllUnhandledErrors(NotFoundViaLegacyLookup.takeError(), errs(),
+        [&LD, LegacyLookup](const SymbolNameSet &Symbols) {
+          auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
+          if (!SymbolFlags) {
+            logAllUnhandledErrors(SymbolFlags.takeError(), errs(),
                                   "CODLayer/SubResolver flags lookup failed: ");
-            SymbolFlags.clear();
-            return SymbolNameSet();
+            return SymbolFlagsMap();
           }
-          return LD.BackingResolver->lookupFlags(SymbolFlags,
-                                                 *NotFoundViaLegacyLookup);
+
+          if (SymbolFlags->size() == Symbols.size())
+            return *SymbolFlags;
+
+          SymbolNameSet NotFoundViaLegacyLookup;
+          for (auto &S : Symbols)
+            if (!SymbolFlags->count(S))
+              NotFoundViaLegacyLookup.insert(S);
+
+          auto SymbolFlags2 =
+              LD.BackingResolver->lookupFlags(NotFoundViaLegacyLookup);
+
+          for (auto &KV : SymbolFlags2)
+            (*SymbolFlags)[KV.first] = std::move(KV.second);
+
+          return *SymbolFlags;
         },
         [this, &LD, LegacyLookup](std::shared_ptr<AsynchronousSymbolQuery> Q,
                                   SymbolNameSet Symbols) {

include/llvm/ExecutionEngine/Orc/Core.h

@@ -577,7 +577,7 @@ public:
 
   /// Search the given VSO for the symbols in Symbols. If found, store
   ///        the flags for each symbol in Flags. Returns any unresolved symbols.
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags, const SymbolNameSet &Names);
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Names);
 
   /// Search the given VSOs in order for the symbols in Symbols. Results
   ///        (once they become available) will be returned via the given Query.

include/llvm/ExecutionEngine/Orc/Legacy.h

@@ -33,8 +33,7 @@ public:
 
   /// Returns the flags for each symbol in Symbols that can be found,
   ///        along with the set of symbol that could not be found.
-  virtual SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
-                                    const SymbolNameSet &Symbols) = 0;
+  virtual SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) = 0;
 
   /// For each symbol in Symbols that can be found, assigns that symbols
   ///        value in Query. Returns the set of symbols that could not be found.
@@ -55,9 +54,8 @@ public:
       : LookupFlags(std::forward<LookupFlagsFnRef>(LookupFlags)),
         Lookup(std::forward<LookupFnRef>(Lookup)) {}
 
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
-                            const SymbolNameSet &Symbols) final {
-    return LookupFlags(Flags, Symbols);
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
+    return LookupFlags(Symbols);
   }
 
   SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -111,21 +109,18 @@ private:
 ///
 /// Useful for implementing lookupFlags bodies that query legacy resolvers.
 template <typename FindSymbolFn>
-Expected<SymbolNameSet> lookupFlagsWithLegacyFn(SymbolFlagsMap &SymbolFlags,
-                                                const SymbolNameSet &Symbols,
-                                                FindSymbolFn FindSymbol) {
-  SymbolNameSet SymbolsNotFound;
+Expected<SymbolFlagsMap> lookupFlagsWithLegacyFn(const SymbolNameSet &Symbols,
+                                                 FindSymbolFn FindSymbol) {
+  SymbolFlagsMap SymbolFlags;
 
   for (auto &S : Symbols) {
     if (JITSymbol Sym = FindSymbol(*S))
       SymbolFlags[S] = Sym.getFlags();
     else if (auto Err = Sym.takeError())
       return std::move(Err);
-    else
-      SymbolsNotFound.insert(S);
   }
 
-  return SymbolsNotFound;
+  return SymbolFlags;
 }
 
 /// Use the given legacy-style FindSymbol function (i.e. a function that
@@ -182,14 +177,12 @@ public:
       : ES(ES), LegacyLookup(std::move(LegacyLookup)),
         ReportError(std::move(ReportError)) {}
 
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
-                            const SymbolNameSet &Symbols) final {
-    if (auto RemainingSymbols =
-            lookupFlagsWithLegacyFn(Flags, Symbols, LegacyLookup))
-      return std::move(*RemainingSymbols);
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) final {
+    if (auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup))
+      return std::move(*SymbolFlags);
     else {
-      ReportError(RemainingSymbols.takeError());
-      return Symbols;
+      ReportError(SymbolFlags.takeError());
+      return SymbolFlagsMap();
     }
   }
 

include/llvm/ExecutionEngine/Orc/NullResolver.h

@@ -23,8 +23,7 @@ namespace orc {
 
 class NullResolver : public SymbolResolver {
 public:
-  SymbolNameSet lookupFlags(SymbolFlagsMap &Flags,
-                            const SymbolNameSet &Symbols) override;
+  SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override;
 
   SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,
                        SymbolNameSet Symbols) override;

lib/ExecutionEngine/Orc/Core.cpp

@@ -522,11 +522,14 @@ ReExportsMaterializationUnit::extractFlags(const SymbolAliasMap &Aliases) {
 
 Expected<SymbolAliasMap>
 buildSimpleReexportsAliasMap(VSO &SourceV, const SymbolNameSet &Symbols) {
-  SymbolFlagsMap Flags;
-  auto Unresolved = SourceV.lookupFlags(Flags, Symbols);
+  auto Flags = SourceV.lookupFlags(Symbols);
 
-  if (!Unresolved.empty())
+  if (Flags.size() != Symbols.size()) {
+    SymbolNameSet Unresolved = Symbols;
+    for (auto &KV : Flags)
+      Unresolved.erase(KV.first);
     return make_error<SymbolsNotFound>(std::move(Unresolved));
+  }
 
   SymbolAliasMap Result;
   for (auto &Name : Symbols) {
@@ -900,22 +903,20 @@ void VSO::removeFromSearchOrder(VSO &V) {
   });
 }
 
-SymbolNameSet VSO::lookupFlags(SymbolFlagsMap &Flags,
-                               const SymbolNameSet &Names) {
+SymbolFlagsMap VSO::lookupFlags(const SymbolNameSet &Names) {
   return ES.runSessionLocked([&, this]() {
-    auto Unresolved = lookupFlagsImpl(Flags, Names);
+    SymbolFlagsMap Result;
+    auto Unresolved = lookupFlagsImpl(Result, Names);
     if (FallbackDefinitionGenerator && !Unresolved.empty()) {
       auto FallbackDefs = FallbackDefinitionGenerator(*this, Unresolved);
       if (!FallbackDefs.empty()) {
-        auto Unresolved2 = lookupFlagsImpl(Flags, FallbackDefs);
+        auto Unresolved2 = lookupFlagsImpl(Result, FallbackDefs);
         (void)Unresolved2;
         assert(Unresolved2.empty() &&
                "All fallback defs should have been found by lookupFlagsImpl");
-        for (auto &D : FallbackDefs)
-          Unresolved.erase(D);
       }
     };
-    return Unresolved;
+    return Result;
   });
 }
 

lib/ExecutionEngine/Orc/Legacy.cpp

@@ -48,8 +48,7 @@ JITSymbolResolverAdapter::lookupFlags(const LookupSet &Symbols) {
   for (auto &S : Symbols)
     InternedSymbols.insert(ES.getSymbolStringPool().intern(S));
 
-  SymbolFlagsMap SymbolFlags;
-  R.lookupFlags(SymbolFlags, InternedSymbols);
+  SymbolFlagsMap SymbolFlags = R.lookupFlags(InternedSymbols);
   LookupFlagsResult Result;
   for (auto &KV : SymbolFlags) {
     ResolvedStrings.insert(KV.first);

lib/ExecutionEngine/Orc/NullResolver.cpp

@@ -14,9 +14,8 @@
 namespace llvm {
 namespace orc {
 
-SymbolNameSet NullResolver::lookupFlags(SymbolFlagsMap &Flags,
-                                        const SymbolNameSet &Symbols) {
-  return Symbols;
+SymbolFlagsMap NullResolver::lookupFlags(const SymbolNameSet &Symbols) {
+  return SymbolFlagsMap();
 }
 
 SymbolNameSet

lib/ExecutionEngine/Orc/OrcCBindingsStack.h

@@ -129,21 +129,20 @@ private:
         : Stack(Stack), ExternalResolver(std::move(ExternalResolver)),
           ExternalResolverCtx(std::move(ExternalResolverCtx)) {}
 
-    orc::SymbolNameSet lookupFlags(orc::SymbolFlagsMap &SymbolFlags,
-                                   const orc::SymbolNameSet &Symbols) override {
-      orc::SymbolNameSet SymbolsNotFound;
+    orc::SymbolFlagsMap
+    lookupFlags(const orc::SymbolNameSet &Symbols) override {
+      orc::SymbolFlagsMap SymbolFlags;
 
       for (auto &S : Symbols) {
         if (auto Sym = findSymbol(*S))
           SymbolFlags[S] = Sym.getFlags();
         else if (auto Err = Sym.takeError()) {
           Stack.reportError(std::move(Err));
-          return orc::SymbolNameSet();
-        } else
-          SymbolsNotFound.insert(S);
+          return orc::SymbolFlagsMap();
+        }
       }
 
-      return SymbolsNotFound;
+      return SymbolFlags;
     }
 
     orc::SymbolNameSet

lib/ExecutionEngine/Orc/OrcMCJITReplacement.h

@@ -144,28 +144,26 @@ class OrcMCJITReplacement : public ExecutionEngine {
   public:
     LinkingORCResolver(OrcMCJITReplacement &M) : M(M) {}
 
-    SymbolNameSet lookupFlags(SymbolFlagsMap &SymbolFlags,
-                              const SymbolNameSet &Symbols) override {
-      SymbolNameSet UnresolvedSymbols;
+    SymbolFlagsMap lookupFlags(const SymbolNameSet &Symbols) override {
+      SymbolFlagsMap SymbolFlags;
 
       for (auto &S : Symbols) {
         if (auto Sym = M.findMangledSymbol(*S)) {
           SymbolFlags[S] = Sym.getFlags();
         } else if (auto Err = Sym.takeError()) {
           M.reportError(std::move(Err));
-          return SymbolNameSet();
+          return SymbolFlagsMap();
         } else {
           if (auto Sym2 = M.ClientResolver->findSymbolInLogicalDylib(*S)) {
             SymbolFlags[S] = Sym2.getFlags();
           } else if (auto Err = Sym2.takeError()) {
             M.reportError(std::move(Err));
-            return SymbolNameSet();
-          } else
-            UnresolvedSymbols.insert(S);
+            return SymbolFlagsMap();
+          }
         }
       }
 
-      return UnresolvedSymbols;
+      return SymbolFlags;
     }
 
     SymbolNameSet lookup(std::shared_ptr<AsynchronousSymbolQuery> Query,

lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp

@@ -64,7 +64,7 @@ public:
         return;
 
       assert(VSOs.front() && "VSOList entry can not be null");
-      VSOs.front()->lookupFlags(InternedResult, InternedSymbols);
+      InternedResult = VSOs.front()->lookupFlags(InternedSymbols);
     });
 
     LookupFlagsResult Result;

unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

@@ -215,11 +215,8 @@ TEST_F(CoreAPIsStandardTest, LookupFlagsTest) {
 
   SymbolNameSet Names({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
-  auto SymbolsNotFound = V.lookupFlags(SymbolFlags, Names);
+  auto SymbolFlags = V.lookupFlags(Names);
 
-  EXPECT_EQ(SymbolsNotFound.size(), 1U) << "Expected one not-found symbol";
-  EXPECT_EQ(SymbolsNotFound.count(Baz), 1U) << "Expected Baz to be not-found";
   EXPECT_EQ(SymbolFlags.size(), 2U)
       << "Returned symbol flags contains unexpected results";
   EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Missing lookupFlags result for Foo";

unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp

@@ -22,17 +22,14 @@ TEST_F(LegacyAPIsStandardTest, TestLambdaSymbolResolver) {
   cantFail(V.define(absoluteSymbols({{Foo, FooSym}, {Bar, BarSym}})));
 
   auto Resolver = createSymbolResolver(
-      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
-        return V.lookupFlags(SymbolFlags, Symbols);
-      },
+      [&](const SymbolNameSet &Symbols) { return V.lookupFlags(Symbols); },
       [&](std::shared_ptr<AsynchronousSymbolQuery> Q, SymbolNameSet Symbols) {
         return V.lookup(std::move(Q), Symbols);
       });
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
-  SymbolNameSet SymbolsNotFound = Resolver->lookupFlags(SymbolFlags, Symbols);
+  SymbolFlagsMap SymbolFlags = Resolver->lookupFlags(Symbols);
 
   EXPECT_EQ(SymbolFlags.size(), 2U)
       << "lookupFlags returned the wrong number of results";
@@ -42,10 +39,6 @@ TEST_F(LegacyAPIsStandardTest, TestLambdaSymbolResolver) {
       << "Incorrect lookupFlags result for Foo";
   EXPECT_EQ(SymbolFlags[Bar], BarSym.getFlags())
       << "Incorrect lookupFlags result for Bar";
-  EXPECT_EQ(SymbolsNotFound.size(), 1U)
-      << "Expected one symbol not found in lookupFlags";
-  EXPECT_EQ(SymbolsNotFound.count(Baz), 1U)
-      << "Expected baz not to be found in lookupFlags";
 
   bool OnResolvedRun = false;
 
@@ -86,9 +79,8 @@ TEST(LegacyAPIInteropTest, QueryAgainstVSO) {
   JITEvaluatedSymbol FooSym(0xdeadbeef, JITSymbolFlags::Exported);
   cantFail(V.define(absoluteSymbols({{Foo, FooSym}})));
 
-  auto LookupFlags = [&](SymbolFlagsMap &SymbolFlags,
-                         const SymbolNameSet &Names) {
-    return V.lookupFlags(SymbolFlags, Names);
+  auto LookupFlags = [&](const SymbolNameSet &Names) {
+    return V.lookupFlags(Names);
   };
 
   auto Lookup = [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
@@ -153,19 +145,14 @@ TEST(LegacyAPIInteropTset, LegacyLookupHelpersFn) {
 
   SymbolNameSet Symbols({Foo, Bar, Baz});
 
-  SymbolFlagsMap SymbolFlags;
-  auto SymbolsNotFound =
-      lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup);
-
-  EXPECT_TRUE(!!SymbolsNotFound) << "lookupFlagsWithLegacy failed unexpectedly";
-  EXPECT_EQ(SymbolFlags.size(), 2U) << "Wrong number of flags returned";
-  EXPECT_EQ(SymbolFlags.count(Foo), 1U) << "Flags for foo missing";
-  EXPECT_EQ(SymbolFlags.count(Bar), 1U) << "Flags for foo missing";
-  EXPECT_EQ(SymbolFlags[Foo], FooFlags) << "Wrong flags for foo";
-  EXPECT_EQ(SymbolFlags[Bar], BarFlags) << "Wrong flags for foo";
-  EXPECT_EQ(SymbolsNotFound->size(), 1U) << "Expected one symbol not found";
-  EXPECT_EQ(SymbolsNotFound->count(Baz), 1U)
-      << "Expected symbol baz to be not found";
+  auto SymbolFlags = lookupFlagsWithLegacyFn(Symbols, LegacyLookup);
+
+  EXPECT_TRUE(!!SymbolFlags) << "Expected lookupFlagsWithLegacyFn to succeed";
+  EXPECT_EQ(SymbolFlags->size(), 2U) << "Wrong number of flags returned";
+  EXPECT_EQ(SymbolFlags->count(Foo), 1U) << "Flags for foo missing";
+  EXPECT_EQ(SymbolFlags->count(Bar), 1U) << "Flags for foo missing";
+  EXPECT_EQ((*SymbolFlags)[Foo], FooFlags) << "Wrong flags for foo";
+  EXPECT_EQ((*SymbolFlags)[Bar], BarFlags) << "Wrong flags for foo";
   EXPECT_FALSE(BarMaterialized)
       << "lookupFlags should not have materialized bar";
 

unittests/ExecutionEngine/Orc/RTDyldObjectLinkingLayerTest.cpp

@@ -184,9 +184,8 @@ TEST_F(RTDyldObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {
   };
 
   Resolvers[K2] = createSymbolResolver(
-      [&](SymbolFlagsMap &SymbolFlags, const SymbolNameSet &Symbols) {
-        return cantFail(
-            lookupFlagsWithLegacyFn(SymbolFlags, Symbols, LegacyLookup));
+      [&](const SymbolNameSet &Symbols) {
+        return cantFail(lookupFlagsWithLegacyFn(Symbols, LegacyLookup));
       },
       [&](std::shared_ptr<AsynchronousSymbolQuery> Query,
           const SymbolNameSet &Symbols) {