[Orc] make getResponsibilitySetWithLegacyFn behavior match with LegacyJITSymbolResolver::getResponsibilitySet

[twoh]

Summary

https://reviews.llvm.org/rL340874 replaces lookupFlags with getResponsibilitySet, and this introduces a behaviral change in LegacyJITSymbolResolver::getResponsibilitySet (If there is no existing definition then the caller is responsible for materializing it. https://github.com/llvm-mirror/llvm/blob/master/lib/ExecutionEngine/RuntimeDyld/JITSymbol.cpp#L127). However, this changes is missed from getResponsibilitySetWithLegacyFn. Though I'm not very familiar with Orc code base, I think the behavior of these two functions supposed to match, and the patch actually fixes "symbols not found" error in our codebase using OrcJIT.

[twoh]

Friendly ping. Thanks!

[lhames]

twoh: Sorry this sat unreviewed for so long. Is this still hitting you in your codebase, or have you moved over to ORCv2?

Since the current plan is to kill off the legacy ORC stuff in ORCv1 I'm inclined to leave things as-is until we delete this code.

include/llvm/ExecutionEngine/Orc/Legacy.h

getResponsibilitySetWithLegacyFn(const SymbolNameSet &Symbols,
   SymbolNameSet Result;
 
   for (auto &S : Symbols) {
     if (JITSymbol Sym = FindSymbol(*S)) {
       if (!Sym.getFlags().isStrong())
         Result.insert(S);
     } else if (auto Err = Sym.takeError())
       return std::move(Err);
+    else {
+      // If there is no existing definition then the caller is responsible for
+      // it.
+      Result.insert(S);
+    }
   }
 
   return Result;
 }
 
 /// Use the given legacy-style FindSymbol function (i.e. a function that
 ///        takes a const std::string& or StringRef and returns a JITSymbol) to
 ///        find the address and flags for each symbol in Symbols and store the

unittests/ExecutionEngine/Orc/LegacyAPIInteropTest.cpp

auto LegacyLookup = [&](const std::string &Name) -> JITSymbol {
 
     return nullptr;
   };
 
   auto RS =
       getResponsibilitySetWithLegacyFn(SymbolNameSet({Bar, Baz}), LegacyLookup);
 
   EXPECT_TRUE(!!RS) << "Expected getResponsibilitySetWithLegacyFn to succeed";
-  EXPECT_EQ(RS->size(), 1U) << "Wrong number of symbols returned";
+  EXPECT_EQ(RS->size(), 2U) << "Wrong number of symbols returned";
   EXPECT_EQ(RS->count(Bar), 1U) << "Incorrect responsibility set returned";
   EXPECT_FALSE(BarMaterialized)
       << "lookupFlags should not have materialized bar";
 
   bool OnResolvedRun = false;
   bool OnReadyRun = false;
   auto OnResolved = [&](Expected<SymbolMap> Result) {
     OnResolvedRun = true;