<div dir="ltr">Yea, sorry, I should have mentioned that I like the simplification of member detection, as long as the 2-stage lookup is still maintained.  i.e. 1) Search for member, if found use it (even if it is found but doesn't compile), if not found then 2) Search for format_provider.</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Dec 12, 2016 at 10:02 AM Pavel Labath via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath planned changes to this revision.<br class="gmail_msg">
labath added a comment.<br class="gmail_msg">
<br class="gmail_msg">
Given that you have commented on implementation details rather than the overall idea, I am going to assume you are ok with this approach. I am going to refine this and get back..<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: include/llvm/Support/FormatVariadicDetails.h:22-23<br class="gmail_msg">
 namespace detail {<br class="gmail_msg">
+class format_adapter {};<br class="gmail_msg">
+}<br class="gmail_msg">
+<br class="gmail_msg">
----------------<br class="gmail_msg">
zturner wrote:<br class="gmail_msg">
> What if we added `virtual void format(llvm::raw_ostream &Stream, StringRef Style) const = 0;` to this class?<br class="gmail_msg">
><br class="gmail_msg">
> This would guaranteed that if the `is_base_of` check ever succeeded, it would never fail to compile because of a missing format member.<br class="gmail_msg">
I wasn't sure how much we cared about the performance impact of a virtual call. I can certainly add it. (Note the code will still fail to compile if the member is missing. The abstract function will give the error message a bit earlier though.)<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: include/llvm/Support/FormatVariadicDetails.h:25-32<br class="gmail_msg">
+template<typename T><br class="gmail_msg">
+class format_adapter: public detail::format_adapter {<br class="gmail_msg">
+  typedef T type;<br class="gmail_msg">
+protected:<br class="gmail_msg">
+  explicit format_adapter(T Item) : Item(Item) {}<br class="gmail_msg">
+<br class="gmail_msg">
+  T Item;<br class="gmail_msg">
----------------<br class="gmail_msg">
zturner wrote:<br class="gmail_msg">
> Do we actually need the templated version of this class?  Why not just have `AdapterBase` inherit directly from `format_adapter`?  It seems like `AdapterBase<T>` and `format_adapter<T>` could be merged into 1 class.<br class="gmail_msg">
Yeah, that can certainly be cleaned up. I am not sure I like the name AdapterBase though. If this is going to be the main entry point for user-written code, I'd rather call it FormatAdapter or Adapter (be it in CamelCase or snake_form).<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27679" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27679</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>