[PATCH] D88450: [ms] [llvm-ml] Accept whitespace around the dot operator

Eric Astor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 29 11:49:39 PDT 2020


epastor marked 3 inline comments as done.
epastor added inline comments.


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1706
+          consumeToken();
+          if (dotOffset < Identifier.size() - 1) {
+            getLexer().UnLex(AsmToken(AsmToken::Identifier,
----------------
rnk wrote:
> My suggestion to try to simplify this code would be to use .slice to build two StringRefs, then check if they are non-empty, and unlex them if needed:
>   StringRef LHS = Identifer.slice(0, dotOffset);
>   StringRef RHS = Identifer.slice(dotOffset+1, Identifer.size());
>   if (LHS.empty())
>     .. UnLex LHS
>   .. Unlex dot
>   if (RHS.empty())
>     .. UnLex RHS
> 
> That keeps all the index math closer together, less spread out.
Done, thanks! (It's RHS first, since UnLex treats the lexer as a stack.)


================
Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:1707
+          if (dotOffset < Identifier.size() - 1) {
+            getLexer().UnLex(AsmToken(AsmToken::Identifier,
+                                      Identifier.substr(dotOffset + 1)));
----------------
rnk wrote:
> Truly, breaking up identifiers into small chunks is the job of the lexer, but here we are anyway, breaking things up and "unlexing" them.
Yep. *sigh*


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88450/new/

https://reviews.llvm.org/D88450



More information about the llvm-commits mailing list