[PATCH] D50839: [llvm] Optimize YAML::isNumeric

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov added a comment.

Mostly LG, just a few more NITs



================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:458
+    ++Digits;
+  return S.drop_front(Digits);
+}
----------------
- Maybe simplify to `return S.dropWhile(...)`?
- Maybe make it a lambda and put inside `isNumeric`?


================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:464
+  // safe.
+  if (S.empty() || S.equals("+") || S.equals("-"))
+    return false;
----------------
Maybe use `S == "+"` instead of `S.equals("+")`?
Just a suggestion, feel free to ignore


================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:497
+  if (S.startswith(".") &&
+      (S.equals(".") || (S.size() > 1 && std::strchr("0123456789",
+                                                     S[1]) == nullptr)))
----------------
maybe use `std::isdigit(S[1])` instead?


================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:530
+      return true;
+    } else if (S.front() == 'e' || S.front() == 'E') {
+      State = FoundExponent;
----------------
NIT: remove braces, remove `else`. LLVM Style guide has a section on it :-)


https://reviews.llvm.org/D50839