<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-06-19 16:40 GMT-07:00 Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Overall, this LGTM. A couple small suggestions in case performance is ever a problem here.<br>
<br>
<br>
REPOSITORY<br>
  rL LLVM<br>
<br>
================<br>
Comment at: lib/CodeGen/MIRParser/MILexer.h:58-60<br>
@@ +57,5 @@<br>
+/// the remaining source string.<br>
+StringRef lexMIToken(<br>
+    StringRef Source, MIToken &Token,<br>
+    std::function<void(StringRef::iterator, const Twine &)> ErrorCallback);<br>
+<br>
----------------<br>
We probably don't want to be passing a std::function here by value. Probably just use a reference to std::function or llvm::function_ref <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_docs_ProgrammersManual.html-23the-2Dfunction-2Dref-2Dclass-2Dtemplate&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=qlzd79BWzssVtJK_rpCPGpeCfhjGJqhUPVzxla-YNLI&s=ZFXtbJFAFlYkoA-w4WtFTBciHB3KWN9hG5woVtR6hC0&e=" rel="noreferrer" target="_blank">http://llvm.org/docs/ProgrammersManual.html#the-function-ref-class-template</a><br>
<br></blockquote><div><br></div><div>Thanks, llvm::function_ref is perfect here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/CodeGen/MIRParser/MIParser.cpp:74-76<br>
@@ +73,5 @@<br>
+void MIParser::lex() {<br>
+  CurrentSource = lexMIToken(<br>
+      CurrentSource, Token,<br>
+      [this](StringRef::iterator Loc, const Twine &Msg) { error(Loc, Msg); });<br>
+}<br>
----------------<br>
Could you doublecheck that the compiler is able to optimize the materialization of the std::function here? If it isn't doing a good job, then it might be better to just construct the std::function once in the MIParser constructor and store it as a member.<br></blockquote><div><br></div><div>The llvm::function_ref materialization is optimized out very effectively by the compiler during the optimized build.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10521&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=qlzd79BWzssVtJK_rpCPGpeCfhjGJqhUPVzxla-YNLI&s=AzRFLIVIMqRx9wf8sykUNhOaeM-HCnTg-EcL_3AhVz8&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/D10521</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=qlzd79BWzssVtJK_rpCPGpeCfhjGJqhUPVzxla-YNLI&s=PGlMQyirtg8fM0pV6FXaV2Bsnf-M7a86lsrUybXYrgs&e=" rel="noreferrer" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</div></div></blockquote></div><br></div></div>