kbobyrev updated this revision to Diff 161204.
kbobyrev added a comment.
Add couple tests, fix formatting issues, use `__builtin_trap()` instead of
`assert` in fuzzer so that it's more transparent.
Also, fuzzing this unreadable version for a couple of hours suggests that it is
valid.
https://
kbobyrev updated this revision to Diff 161192.
kbobyrev added a comment.
I tried to rewrite the loop, but IMO it looks even worse now.
https://reviews.llvm.org/D50839
Files:
llvm/include/llvm/Support/YAMLTraits.h
llvm/tools/llvm-yaml-numeric-parser-fuzzer/CMakeLists.txt
llvm/tools/llvm-ya
kbobyrev updated this revision to Diff 161191.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Upload version which is IMO readable.
https://reviews.llvm.org/D50839
Files:
llvm/include/llvm/Support/YAMLTraits.h
llvm/tools/llvm-yaml-numeric-parser-fuzzer/CMakeLists.txt
kbobyrev added inline comments.
Comment at: llvm/include/llvm/Support/YAMLTraits.h:454
+inline bool isNumeric(StringRef S) {
+ if (S.empty())
+return false;
zturner wrote:
> What would happen if we re-wrote this entire function as:
>
> ```
> inline bool isN
zturner added inline comments.
Comment at: llvm/include/llvm/Support/YAMLTraits.h:454
+inline bool isNumeric(StringRef S) {
+ if (S.empty())
+return false;
What would happen if we re-wrote this entire function as:
```
inline bool isNumeric(StringRef S) {
lebedev.ri added inline comments.
Comment at: llvm/unittests/Support/YAMLIOTest.cpp:2626
+ //
+ // * Sexagecimal numbers
+ // * Decimal numbers with comma s the delimiter
Spelling
Comment at: llvm/unittests/Support/YAMLIOTest.cpp:2627
+ //
joerg added inline comments.
Comment at:
llvm/tools/llvm-yaml-numeric-parser-fuzzer/yaml-numeric-parser-fuzzer.cpp:16
+
+llvm::Regex Inifnity("^[-+]?(\\.inf|\\.Inf|\\.INF)$");
+llvm::Regex Base8("^0o[0-7]+$");
Spelling?
https://reviews.llvm.org/D50839
_
kbobyrev updated this revision to Diff 161033.
kbobyrev added a comment.
Use consistent `Regex` matchers naming: don't append "Matcher" at the end.
https://reviews.llvm.org/D50839
Files:
llvm/include/llvm/Support/YAMLTraits.h
llvm/tools/llvm-yaml-numeric-parser-fuzzer/CMakeLists.txt
llvm/
kbobyrev updated this revision to Diff 161027.
kbobyrev marked 2 inline comments as done.
kbobyrev added a subscriber: lebedev.ri.
kbobyrev added a comment.
Herald added a subscriber: mgorny.
Very good point by @lebedev.ri! I have added a very simple fuzzer for the
parser. So far, there were no i
joerg added inline comments.
Comment at: llvm/include/llvm/Support/YAMLTraits.h:477
+ if (ParseOct && std::find(std::begin(OctalChars), std::end(OctalChars),
+Char) == std::end(OctalChars))
+return false;
Can you use s
ilya-biryukov added a comment.
Just drive-by comments, the maintainers of the code are in a much better
position to give feedback, of course.
Nevertheless, my few cents:
- Getting rid of a regex in favor of the explicit loop is definitely a good
thing. It's incredibly how much time is spent the
lebedev.ri added inline comments.
Comment at: llvm/include/llvm/Support/YAMLTraits.h:453
-inline bool isNumber(StringRef S) {
- static const char OctalChars[] = "01234567";
- if (S.startswith("0") &&
- S.drop_front().find_first_not_of(OctalChars) == StringRef::npos)
-
kbobyrev created this revision.
kbobyrev added reviewers: ilya-biryukov, ioeric.
Herald added a reviewer: javed.absar.
Herald added a subscriber: kristof.beyls.
This patch significantly improves performance of the YAML serializer by
optimizing `YAML::isNumeric` function. This function is called o
13 matches
Mail list logo
