[PATCH] MIR Parsing: Introduce a MI Lexing class.
Sean Silva
chisophugis at gmail.com
Fri Jun 19 16:40:51 PDT 2015
Overall, this LGTM. A couple small suggestions in case performance is ever a problem here.
REPOSITORY
rL LLVM
================
Comment at: lib/CodeGen/MIRParser/MILexer.h:58-60
@@ +57,5 @@
+/// the remaining source string.
+StringRef lexMIToken(
+ StringRef Source, MIToken &Token,
+ std::function<void(StringRef::iterator, const Twine &)> ErrorCallback);
+
----------------
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 http://llvm.org/docs/ProgrammersManual.html#the-function-ref-class-template
================
Comment at: lib/CodeGen/MIRParser/MIParser.cpp:74-76
@@ +73,5 @@
+void MIParser::lex() {
+ CurrentSource = lexMIToken(
+ CurrentSource, Token,
+ [this](StringRef::iterator Loc, const Twine &Msg) { error(Loc, Msg); });
+}
----------------
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.
http://reviews.llvm.org/D10521
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list