[PATCH] D84050: YAML: Don't assume an arbitrary StringRef is null terminated

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Support/YAMLParser.cpp:1410-1411
 bool Scanner::scanAliasOrAnchor(bool IsAlias) {
-  StringRef::iterator Start = Current;
   unsigned ColStart = Column;
+  StringRef::iterator Start = Current;
   skip(1);
----------------
I don't have an opinion on the best order of these two lines, but if you want to flip them please do so in a separate NFC commit (and revert the change from this patch to make it as clean as possible).


================
Comment at: llvm/lib/Support/YAMLParser.cpp:1413
   skip(1);
-  while(true) {
+  while(Current != End) {
     if (   *Current == '[' || *Current == ']'
----------------
Please run `git-clang-format` or similar to fix the spacing on the lines touched.


================
Comment at: llvm/lib/Support/YAMLParser.cpp:1426
 
-  if (Start == Current) {
+  if (Start + 1 == Current) {
     setError("Got empty alias or anchor", Start);
----------------
This looks like a fix for a logic bug, not directly related to null-termination; can it be committed separately ahead of time? Please also add a test that covers it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84050