This is an archive of the discontinued LLVM Phabricator instance.

Allow SmallVector to be used as a yaml sequence
ClosedPublic

Authored by zturner on Aug 5 2016, 11:00 AM.

Details

Summary

Currently YAML sequences require std::vectors. All of the methods that the YAML parser accesses though are present in SmallVector, so there's no reason we can't support SmallVector inherently. This patch does that.

Diff Detail

Event Timeline

zturner updated this revision to Diff 66979.Aug 5 2016, 11:00 AM
zturner retitled this revision from to Allow SmallVector to be used as a yaml sequence.
zturner updated this object.
zturner added reviewers: majnemer, arphaman, kledzik.
zturner added a subscriber: llvm-commits.
majnemer added inline comments.Aug 5 2016, 11:05 AM
include/llvm/Support/YAMLTraits.h
1362–1382

Is this formatted right? it looks funny to me for some reason.

1381

Where is flow used?

1394

Shouldn't this be unsigned to match SmallVector's template?

1412

Ditto.

zturner added inline comments.Aug 5 2016, 11:31 AM
include/llvm/Support/YAMLTraits.h
1362–1382

It's clang-formatted, so I guess so. Which party looks funny?

1381

It seems to be used to distinguish a flow sequence from a regular sequence. I don't know how it's used, other than to say that it's an optional member that if defined, causes the parser to treat it differently. See line 47 of this file.

That said, FlowSequenceTraitsImpl and SequenceTraitsImpl are identical with the exception of the existence of the flow variable. So I'm going to remove this class and consolidate. New patch incoming shortly.

1394

Yes, good catch.

zturner updated this revision to Diff 66985.Aug 5 2016, 11:32 AM
majnemer accepted this revision.Aug 5 2016, 11:58 AM
majnemer edited edge metadata.

LGTM with nits.

include/llvm/Support/YAMLTraits.h
1362

My clang-format would put this on its own line.

This revision is now accepted and ready to land.Aug 5 2016, 11:58 AM
This revision was automatically updated to reflect the committed changes.