This is an archive of the discontinued LLVM Phabricator instance.

Add Printable class to simplify the construction of Print helpers.
ClosedPublic

Authored by MatzeB on Nov 4 2015, 1:36 PM.

Details

Summary

Printable objects simply hold a function reference to a function that
does the printing. It allows the construction of Print helpers.

This would also be a good base to transform several MyClass::print(raw_ostream &OS, more arguments); function in llvm to a more C++-style interface (though I do not plan to do this refactoring myself at the moment).

Example:

Printable PrintLaneMask(unsigned LaneMask) {
  return Printable([LaneMask](raw_ostream &OS) {
    OS << format("%08X", LaneMask);
  });
}

// Usage:
OS << PrintLaneMask(Mask);

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 39250.Nov 4 2015, 1:36 PM
MatzeB retitled this revision from to Add Printable class to simplify the construction of Print helpers..
MatzeB updated this object.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.

ping2.

Adding some reviewers that might care about the printers here or proper C++ usage.

dblaikie edited edge metadata.Dec 3 2015, 10:12 AM

Perhaps we could skip the Printable type and make this more like IO manipulators in the standard streams ( http://www.cplusplus.com/reference/ostream/ostream/operator%3C%3C/ )

Which is to say, add a "operator<<(std::function<void(raw_ostream&)>)" to raw_ostream, and it just calls it on itself.

Seem reasonable?

MatzeB updated this revision to Diff 41799.Dec 3 2015, 1:52 PM
MatzeB edited edge metadata.

Updated version which uses std::function<void(raw_ostream&)> similar to C++ IO manipulators directly instead of adding a pointless shim class around the std::function.
I still found the code slightly easier to read when giving the concept an explicit name with "typedef std::function<void(raw_ostream&)> Printable", though I have no strong opinion here and can change that if you prefer the plain form without typedef.

dblaikie accepted this revision.Dec 3 2015, 1:57 PM
dblaikie edited edge metadata.

Seems good to me

Don't mind the typedef one way or another terribly much (I usually err towards not typedef'ing relatively obvious/clear things, so the code is less opaque, but it seems OK-ish here)

This revision is now accepted and ready to land.Dec 3 2015, 1:57 PM
This revision was automatically updated to reflect the committed changes.
MatzeB added a comment.Dec 3 2015, 3:29 PM

I had to revert this patch because two gcc buildbots and all the msvc bots choke on ambiguous operator<< overloads. Examples:

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/12622/steps/build%20fresh%20clang/logs/stdio
http://lab.llvm.org:8011/builders/polly-amd64-linux/builds/36852/steps/build_polly/logs/stdio
http://lab.llvm.org:8011/builders/lld-x86_64-win7/builds/27400/steps/build_Lld/logs/stdio

My current guess is that std::function implicitely converts to void* or void* implicitely converts to std::function on these systems. So I guess to get this working I have to go back to the shim class after all :-(