[PATCH] D33899: Allow VersionPrinter to print to arbitrary raw_ostreams

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: include/llvm/Support/CommandLine.h:69
+typedef void (*VersionPrinterTy)(raw_ostream &);
+
 ///===---------------------------------------------------------------------===//
----------------
dim wrote:
> mehdi_amini wrote:
> > dim wrote:
> > > mehdi_amini wrote:
> > > > Using a `std::function` (or maybe `llvm::function_ref`) would be allowing for more rich object to be passed in (i.e. a client that may want state to be passed around).
> > > Maybe, but how would `VersionPrinter::print` or `VersionPrinter::operator=` know how to call that function object, and which parameters to pass?
> > > 
> > > In any case it looks like `llvm::function_ref` also doesn't support variable function types, you'd still need to settle on `function_ref<void(raw_ostream &)>` as 'the' type for version printing functions.  With `std::function` it seems to be the same.
> > > 
> > > The only advantage would maybe be the ownership of the function object, but I'm not sure if that is a big deal breaker in this case.  It's just for printing a version field. :)
> > It is not about the function signature, it is about being able to have a callback with *state*. 
> > 
> > i.e. what people do in C is adding a `void *` parameter to the callback signature and have the caller passing it in alongside the callback, C++ has std::function for this.
> > 
> Sure, but in this case the callback signature has a `raw_ostream &`.  Did you mean you wanted the following:
> 
>     typedef void (*VersionPrintTy)(std::function<whatever> &);
> 
> ? I'm not sure how that would be handy, though.
No I meant something like:

```
typedef std::function<void(raw_ostream &>)> VersionPrintTy;
```


https://reviews.llvm.org/D33899