This is an archive of the discontinued LLVM Phabricator instance.

Pass CVRecords through visitors as non-const references
ClosedPublic

Authored by zturner on Sep 8 2016, 1:01 PM.

Details

Summary

This simplifies a lot of code, and will actually be necessary for an upcoming patch to add a hash calcluation visitor.

The idea before was that visitors should be examining records, not modifying them. But this is no longer true with a visitor which constructs a CVRecord from a yaml description. To handle this until now, we were doing some fixup on the CVRecord objects at a higher level, but it makes a lot more sense to just have the visitor fill out the fields of the CVRecord directly. In doing so I uncovered a few bugs related to Data and RawData and fixed those.

Diff Detail

Event Timeline

zturner updated this revision to Diff 70744.Sep 8 2016, 1:01 PM
zturner retitled this revision from to Pass CVRecords through visitors as non-const references.
zturner updated this object.
zturner added reviewers: rnk, amccarth.
zturner added a subscriber: llvm-commits.
rnk accepted this revision.Sep 8 2016, 2:37 PM
rnk edited edge metadata.

lgtm While you're at it, it would be good if we could kill off Data and RawData, they're pretty redundant. Probably worth a separate change.

This revision is now accepted and ready to land.Sep 8 2016, 2:37 PM
In D24362#537664, @rnk wrote:

lgtm While you're at it, it would be good if we could kill off Data and RawData, they're pretty redundant. Probably worth a separate change.

I tried, but it's not that easy. I still want to think about how to do that some more. The problem is that for top level records, Data = RawData.drop_front(sizeof(RecordPrefix)), but for field list member records, they're equal.

Also, I can't submit this until after D24316 which unfortunately is pretty big. So if someone gets a chance to look at it, would be nice.

This revision was automatically updated to reflect the committed changes.