[PATCH] D62381: Use SymbolTable::insert() to implement --trace.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu marked an inline comment as done.
ruiu added inline comments.


================
Comment at: lld/ELF/SymbolTable.h:38
 
-  ArrayRef<Symbol *> getSymbols() const { return SymVector; }
+  void forEachSymbol(std::function<void(Symbol *)> Fn) {
+    for (Symbol *Sym : SymVector)
----------------
MaskRay wrote:
> MaskRay wrote:
> > grimar wrote:
> > > ruiu wrote:
> > > > grimar wrote:
> > > > > `llvm::function_ref` should probably be a bit better, btw.
> > > > I didn't take a look at the assembly, but isn't this something that a compiler can optimize and erases std::function? Since this is an inline function, a compiler knows exactly how a given lambda is used.
> > > I do not know the answer.
> > `function_ref` should be better, but I don't know if compilers can optimize out the heap allocation used for the lambda.
> > 
> > I still want to understand how complicated the signature of `llvm::make_filter_range` is. Let me check
> 
> ```
> struct FilterOutPlaceholder {
>   bool operator()(Symbol *S) const {
>     return !S->isPlaceholder();
>   }
> };
> 
> static iterator_range<filter_iterator<std::vector<Symbol *>::iterator, FilterOutPlaceholder>> symbols() {
>   return make_filter_range(Symtab->SymVector, FilterOutPlaceholder());
> }
> ```
> 
> The benefit is that the backtrace will be simpler in a debugger.
We can do that too, but it seems a bit more complicated than the lambda.

I'm also making a change to split SymVector and SymMap into multiple shards for parallel name resolution. With that, the iterator would have more complicated type, as you need to concatenate multiple iterators and then filter out some members after that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62381/new/

https://reviews.llvm.org/D62381