[PATCH] Add observers to Input Graph

Shankar Kalpathi Easwaran shankarke at gmail.com
Tue May 13 06:14:23 PDT 2014


Can you also add a test ? Possibly two tests ? One with InputGraphTest and one with YAML ?

================
Comment at: include/lld/Core/InputGraph.h:74
@@ +73,3 @@
+  /// being returned.
+  void registerObserver(std::function<void(File *)> &fn);
+
----------------
Michael Spencer wrote:
> Simon Atanasyan wrote:
> > Maybe it is better to use the `llvm::function_ref` class to be consistent with main LLVM code.
> > 
> > http://llvm.org/docs/ProgrammersManual.html#passing-functions-and-other-callable-objects
> This is news to me. I would much prefer we use std::function.
> 
> So after doing some research, it seems that advanced uses of std::function can fail on VC11 (VS 2012). We're not doing that here, and the buildbots have the final say in what is supported.
You might want to change the observer function to return an ErrorOr,

================
Comment at: lib/ReaderWriter/FileArchive.cpp:100-108
@@ -99,1 +99,11 @@
 
+  /// Returns a set of all defined symbols in the archive.
+  std::set<StringRef> getDefinedSymbols() const override {
+    std::set<StringRef> ret;
+    for (const auto &e : _symbolMemberMap) {
+      StringRef sym = e.first;
+      ret.insert(sym);
+    }
+    return ret;
+  }
+
----------------
This may be put in one line using std::transform.

std::transform(_symbolMemberMap.begin(),_symbolMemberMap.end(), std::back_inserter(ret), [](const auto &pair){return pair.first;});

http://reviews.llvm.org/D3735






More information about the llvm-commits mailing list