[PATCH] D33323: [llvm-pdbdump] Add the ability to merge PDBs

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/tools/llvm-pdbdump/llvm-pdbdump.cpp:1019
+    }
+    mergePdbs();
   }
----------------
inglorion wrote:
> zturner wrote:
> > inglorion wrote:
> > > The other subcommands take function arguments, instead of accessing the globals. Can we do that here as well?
> > Actually it's a little bit of both.  There are so many options that it's impractical to pass the entire set of options to every subcommand's implementation, and they frequently access the globals.  In this instance I did it this way because passing a reference to a `cl::list<std::string>` felt a little awkward.  In the other cases we don't have this problem because there are a fixed number of input files, but in this case there can be arbitrarily many.
> You're right, the others use a combination of arguments and globals. I'd still prefer it if you made this one an argument. I agree that passing a reference to a cl::list feels a little awkward. How about an ArrayRef? IIRC, you can get those out of cl::lists using the operator ArrayRef.
Didn't realize there was an operator ArrayRef.  If that's the case, I agree that seems like a good solution.


https://reviews.llvm.org/D33323