Page MenuHomePhabricator

[StringExtras] Add a helper class for comma-separated lists
ClosedPublic

Authored by kazu on Sun, Jan 10, 1:23 PM.

Details

Summary

This patch introduces a helper class SubsequentDelim to simplify loops
that generate a comma-separated lists.

For example, consider the following loop, taken from
llvm/lib/CodeGen/MachineBasicBlock.cpp:

for (auto I = pred_begin(), E = pred_end(); I != E; ++I) {
  if (I != pred_begin())
    OS << ", ";
  OS << printMBBReference(**I);
}

The new class allows us to rewrite the loop as:

SubsequentDelim SD;
for (auto I = pred_begin(), E = pred_end(); I != E; ++I)
  OS << SD << printMBBReference(**I);

where SD evaluates to the empty string for the first time and ", " for
subsequent iterations.

Unlike interleaveComma, defined in llvm/include/llvm/ADT/STLExtras.h,
SubsequentDelim can accommodate a wider variety of loops, including:

  • those that conditionally skip certain items,
  • those that need iterators to call getSuccProbability(I), and
  • those that iterate over integer ranges.

As an example, this patch cleans up MachineBasicBlock::print.

Diff Detail

Event Timeline

kazu created this revision.Sun, Jan 10, 1:23 PM
kazu requested review of this revision.Sun, Jan 10, 1:23 PM
Herald added a project: Restricted Project. · View Herald TranscriptSun, Jan 10, 1:23 PM
dblaikie accepted this revision.Sun, Jan 10, 2:18 PM

Sure, sounds alright.

Can do similar things with interleaveComma + various filtering and adapting iterators, but for many cases that's probably overly complicated.

This revision is now accepted and ready to land.Sun, Jan 10, 2:18 PM
This revision was automatically updated to reflect the committed changes.
MaskRay added a subscriber: MaskRay.EditedMon, Jan 11, 8:02 PM

The name SubsequentDelim looks a bit strange to me. Maybe others have suggestions on the name?
(personally I might choose AddDelimUnlessFirst)

The name SubsequentDelim looks a bit strange to me. Maybe others have suggestions on the name?
(personally I might choose AddDelimUnlessFirst)

Yeah, I don't have any particularly compelling names - maybe "StreamableSeparator"

kazu added a comment.Tue, Jan 12, 8:59 AM

The name SubsequentDelim looks a bit strange to me. Maybe others have suggestions on the name?
(personally I might choose AddDelimUnlessFirst)

Yeah, I don't have any particularly compelling names - maybe "StreamableSeparator"

How about ListSeparator? Windows seems to have regional setting called "List Separator" specifying what separator to use in a list, so I am not inventing a new term at least:

https://support.talkdesk.com/hc/en-us/articles/360047099811-CSV-Files-and-Regional-Settings

I'd like to keep the name reasonably short, so I might sweep under the carpet the fact that the first invocation generates the empty string.