[PATCH] D70505: [ELF] Replace SymbolTable::forEachSymbol with filter_iterator begin()/end()

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D70505#1754462 <https://reviews.llvm.org/D70505#1754462>, @dblaikie wrote:

> In D70505#1754388 <https://reviews.llvm.org/D70505#1754388>, @ruiu wrote:
>
> > This is my personal taste, but I feel like the former code is simpler, though it doesn't look elegant. The new code looks a bit too smart to me. I also have a little concern to define start and end to SymbolTable class because it is not very clear what are iterated. SymbolTable contains not only symbols but other members.
>
>
> Would it alleviate your concerns if it were more like:
>
>   for (Symbol *sym : symtab->symbols()) 
>   
>
> ?


`symbols()` was my original design, but I felt that `SymbolTable::{begin,end}` is sufficient and easier. This class is called `SymbolTable`, a collection of symbols. It is intuitive to get a symbol when iterating it. I can't think of a second way to iterate it :) This is similar to `tools/llvm-objcopy/ELF/Object.h:SectionTableRef`, which has begin/end as its member functions and allows iterating itself. I am open to a change to `symbols()`, but I wanted to express my preference for begin()/end().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70505