[PATCH] D46699: [ThinLTO] Print module summary index to assembly

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/AsmParser/LLLexer.h:68-69
     void Warning(const Twine &Msg) const { return Warning(getLoc(), Msg); }
+    // FIXME: Make private again when summary parsing support is complete.
+    void SkipLineComment();
 
----------------
dexonsmith wrote:
> tejohnson wrote:
> > dexonsmith wrote:
> > > Rather than making this (temporarily) public, it might be nicer to add a function like "LexModuleSummaryIndex" that calls `SkipLineComment`.  Then it'll be less likely that `SkipLineComment` grows non-`private` users.
> > I realized when working on the parsing that it is going to be useful to keep this functionality public, because for parsing clients that aren't interested in the index, for which we will pass in a null Index pointer, we want to continue to skip the index lines. Similarly, for clients that aren't interested in the IR (e.g. the Thin Link, which when reading a bitcode file skips past all of the IR records), we will pass the parser a null Module pointer and want to skip all the non-summary IR lines. I can add a new public function to call this, not sure of the best name, or just rename SkipLineComment to SkipLine.
> Or perhaps a more honest name like `SkipModuleSummaryIndex`.
I can add that for this patch. But that isn't a good name for the other use case I am adding to my parsing patch, of skipping the IR when we only care about reading the summary index. Should we have two interfaces (e.g. SkipModuleSummaryIndex and SkipModuleIR) or a single generic skip interface?


Repository:
  rL LLVM

https://reviews.llvm.org/D46699