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

Michael Spencer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 11 16:38:23 PST 2020


Bigcheese added a comment.

In D84050#2315437 <https://reviews.llvm.org/D84050#2315437>, @dexonsmith wrote:

> In D84050#2242755 <https://reviews.llvm.org/D84050#2242755>, @arsenm wrote:
>
>> In D84050#2189370 <https://reviews.llvm.org/D84050#2189370>, @bkramer wrote:
>>
>>> The parser used to rely on the `\0` being present. Was that fixed?
>>
>> Do you know where this assumption was? I couldn't find it, but also couldn't find any commits that seemed to "fix" it. I haven't seen a sanitizer error yet
>
> I just did a quick scan and I can't find any such assumption either. @Bigcheese, do remember what the original intent was?

All I recall at this point is that I required a null terminator for the same reason Clang does, to remove the constant need to check for `Current != End`. This probably makes a lot more sense for Clang to do as it's rare to compile source code not in a file, which may not always be true for the YAML parser. I'm fine with removing that assumption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84050



More information about the llvm-commits mailing list