[PATCH] D137118: [YAML] Trim trailing whitespace from plain scalars

[Scott Linder via Phabricator via llvm-commits]

scott.linder accepted this revision.
scott.linder added a comment.
This revision is now accepted and ready to land.

Sorry for the delay in reviewing! LGTM with a couple nits



================
Comment at: llvm/lib/Support/YAMLParser.cpp:2044
   }
   // Plain or block.
+  // Trim whitespace ('b-char' and 's-white').
----------------
Nit: I believe this is just a stale comment from the partial implementation of block scalars before https://reviews.llvm.org/D9503

Could you remove the mention of "block" while you're at it?


================
Comment at: llvm/lib/Support/YAMLParser.cpp:2045
   // Plain or block.
-  return Value.rtrim(' ');
+  // Trim whitespace ('b-char' and 's-white').
+  return Value.rtrim("\x0A\x0D\x20\x09");
----------------
Could you mention the alternative of handling this in the scanner? I am not certain this is the right place for this, but if it is moving us closer to the spec I think it is OK regardless. The YAML spec is so vast and impenetrable that I don't suspect we will ever be fully compliant anyway :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137118