[PATCH] D20876: [PDB] Print section contributions and section map

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: tools/llvm-pdbdump/llvm-pdbdump.cpp:518
@@ +517,3 @@
+  ListScope L(P, "Section Contributions");
+  class Visitor : public ISectionContribVisitor {
+  public:
----------------
ruiu wrote:
> zturner wrote:
> > ruiu wrote:
> > > Define this class outside of this function.
> > > 
> > > Also this class definition made me think of the visitor design choice. Currently visitor's callback is a class with visit() member function. But it can be just a std::function, no? We can create a lambda here and pass it to visitSectionContributions function.
> > The problem is we don't know whether it will be called back with a `SectionContrib` or a `SectionContrib2`.  If it were possible to make a "multi lambda" or something that supported overloads, then yes we could do that.  Of course, I could raise the logic outside and use a switch statement, but in general I like the pattern of handling variant-data with a visitor.
> > 
> > Is there a reason to prefer defining the class outside the function?  This is effectively a lambda with an overload, so just like your lambda would be scoped to the function, I wrote the class the same way.
> OK, if there's a chance for the class to have more than two callback functions, using a class makes sense to me. But currently it has only one callback function, so I'd do with a simple lambda rather than a class to avoid overdesining. I guess it's your call.
> 
> No particular reason to not avoid defining classes inside functions, but it is at least rather unusual in LLVM. If it is a deliberate choice and if you like this, it's OK to me.
It already has 2 functions.  `visit` is overloaded to take a `const SectionContrib &` and a `const SectionContrib2 &`.  See below this function, where the overload delegates to this version of the function and then prints out additional data.


http://reviews.llvm.org/D20876