[PATCH] Move ownership of GCStrategy objects to LLVMContext

[Sanjoy Das]

Some minor nits inline.  I think the ownership change is okay, but I'm not confident enough in that judgement to LGTM it.


================
Comment at: lib/IR/Function.cpp:393
@@ +392,3 @@
+  const char* Name = getGC();
+  LLVMContextImpl* pImpl = getParent()->getContext().pImpl;
+  return pImpl->getGCStrategy(Name);
----------------
LLVM style says `LLVMContextImpl *pImpl` and `const char *Name`.

================
Comment at: lib/IR/Function.cpp:393
@@ +392,3 @@
+  const char* Name = getGC();
+  LLVMContextImpl* pImpl = getParent()->getContext().pImpl;
+  return pImpl->getGCStrategy(Name);
----------------
sanjoy wrote:
> LLVM style says `LLVMContextImpl *pImpl` and `const char *Name`.
Why not just `getContext().pImpl`?

================
Comment at: lib/IR/LLVMContextImpl.cpp:177
@@ +176,3 @@
+
+GCStrategy *LLVMContextImpl::getGCStrategy(const StringRef &Name) {
+  // TODO: Arguably, just doing a linear search would be faster for small N
----------------
`ProgrammersManual.rst` says "``StringRef`` is small and pervasive enough in LLVM that it should always be passed by value."

================
Comment at: lib/IR/LLVMContextImpl.cpp:188
@@ +187,3 @@
+      S->Name = Name;
+      GCStrategyMap[Name] = S.get();
+      GCStrategyList.push_back(std::move(S));
----------------
Not part of the review, but is this complexity (the `StringMap`) actually needed?  I'd think the usual case has just one GC strategy.

http://reviews.llvm.org/D6811

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/