[PATCH] D45948: [MIRParser] Allow register class names in the form of integer/scalar

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin added inline comments.


================
Comment at: llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp:1307
+  if (Token.range().front() == 's' || Token.range().front() == 'p')
+    if (!llvm::all_of(Token.range().drop_front(), isdigit))
+      return error("Expected integers after 's'/'p' type character");
----------------
This check will pass if the token is a single character `s` or `p` identifier, and it probably shouldn't.


================
Comment at: llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp:1310
+  if (!llvm::all_of(Token.range().drop_front(), isdigit))
+    return error("Expected integers after 's'/'p' type character");
+
----------------
This doesn't feel right. It was probably supposed to be a single check under the `s`/`p` condition, the following copy shouldn't be here, I think.


================
Comment at: llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp:1342
     return error(Loc, "expected '<N x sM>' for vector type");
+  if (!llvm::all_of(Token.range().drop_front(), isdigit))
+    return error("Expected integers after 's' type character");
----------------
This check will pass if the token is a single character `s` identifier, and it probably shouldn't.


================
Comment at: llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp:1357
+  assert(Token.is(MIToken::Identifier));
+  if (!llvm::all_of(Token.range().drop_front(), isdigit))
+    return error("Expected integers after 'i'/'s'/'p' type character");
----------------
This check will pass if the token is a single character identifier, and it probably shouldn't.


================
Comment at: llvm/trunk/lib/CodeGen/MIRParser/MIParser.cpp:1358
+  if (!llvm::all_of(Token.range().drop_front(), isdigit))
+    return error("Expected integers after 'i'/'s'/'p' type character");
+
----------------
This message would be confusing if the first character is not `i`, `s`, or `p`.


Repository:
  rL LLVM

https://reviews.llvm.org/D45948