<div dir="ltr"><br><div class="gmail_quote"><div>On Tue, Jun 6, 2017 at 3:26 PM Peter Collingbourne via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">pcc added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/include/llvm/Object/<wbr>IRSymtab.h:123<br>
+/// Fills in Symtab and Strtab with a valid symbol and string table for Mods.<br>
+Error write(ArrayRef<Module *> Mods, SmallVector<char, 0> &Symtab,<br>
+            SmallVector<char, 0> &Strtab);<br>
----------------<br>
pcc wrote:<br>
> tejohnson wrote:<br>
> > The name "write" here seems unexpected to me, since we aren't writing to disk e.g.. The client does a "write" which involves a Writer class, followed by a Reader, when together both are needed to essentially "read" the symbols from Modules. Maybe "buildSymbolTable" or something like that. The Writer is more like a Builder.<br>
> Renamed to "build" (likewise to "Builder").<br>
Every time I go into this code it bugs me a little that we have a reader and a builder (as opposed to a writer). I hate to bring up a bikeshedding topic, and it's not a big deal I guess, but would you mind if I rename this back to write/Writer? In support of my position I point to the many Writer classes we have in LLVM [1] that are not necessarily writing to disk.</blockquote><div><br></div><div>To me a "writer" is doing some sort of serialization while a builder is constructing structured representation / program entities. There is not necessarily a disk involved. </div><div>So conceptually are you "building a SymTab from a Module" or "Writing a Module into a SymTab"?</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div></div></div>