[PATCH] D114223: Make ScopedPrinter interface virtual

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Support/ScopedPrinter.h:164
 
-    llvm::sort(SetFlags, &flagName<TFlag>);
-
-    startLine() << Label << " [ (" << hex(Value) << ")\n";
-    for (const auto &Flag : SetFlags) {
-      startLine() << "  " << Flag.Name << " (" << hex(Flag.Value) << ")\n";
-    }
-    startLine() << "]\n";
+    sort(SetFlags, &flagName);
+    printFlagsImpl(Label, hex(Value), SetFlags);
----------------
I've just remembered that it's common practice to leave the explicit `llvm::` prefix on cases where the function is one of the standard algorithm-like functions, so it's probably best to keep it here.


================
Comment at: llvm/include/llvm/Support/ScopedPrinter.h:375
+
+  virtual void printRawFlagsImpl(StringRef Label, HexNumber Value,
+                                 ArrayRef<FlagEntry> Flags) {
----------------
I'm not a fan of this function name. In some ways, it's less raw than the regular `printFlags` code, as it includes names. Any particular need for it to be a different name, or could we just make use of function overloading?


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

https://reviews.llvm.org/D114223