[PATCH] D85180: [YAMLTraits] Fix mapping <none> value that followed by comments.

[George Rimar via Phabricator via llvm-commits]

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

LGTM with a nit.



================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:1631-1632
     if (!outputting())
       if (auto *Node = dyn_cast<ScalarNode>(((Input *)this)->getCurrentNode()))
-        IsNone = Node->getRawValue() == "<none>";
+        IsNone = Node->getRawValue().trim() == "<none>";
 
----------------
grimar wrote:
> Higuoxing wrote:
> > Another approach would be using `getValue(SmallVectorImpl<char> &Storage)`.
> > 
> > ```
> >    /// Gets the value of this node as a StringRef.
> >    ///
> >    /// \param Storage is used to store the content of the returned StringRef iff
> >    ///        it requires any modification from how it appeared in the source.
> >    ///        This happens with escaped characters and multi-line literals.
> >    StringRef getValue(SmallVectorImpl<char> &Storage) const;
> > ```
> > 
> > ```
> > Flags: [[FLAGS='<none>']] can also be correctly parsed.
> > ```
> I'd probably just stop on on `rtrim(` `)` as it does exactly what we need here.
> 
> `ScalarNode::getValue(SmallVectorImpl<char> &Storage)` also handles single and double quotes,
> what doesn't seem like something what we want to support.
nit: still missing a comment. I'd add something like:

// We use rtrim to ignore possible white spaces that might exist when a comment is present on the same line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85180