From 5301d5347a31d3f5d20b582987eee0cd8c92722e Mon Sep 17 00:00:00 2001
From: cousteaulecommandant <cousteaulecommandant@gmail.com>
Date: Tue, 10 Jan 2017 18:44:02 +0100
Subject: [PATCH 01/19] Inline println(...) functions

Remove all definitions of println(...) from Print.cpp except println(void),
and replace them with inline methods in Print.h.
This simplifies the code in Print.cpp improving maintainability.
---
 cores/arduino/Print.cpp | 77 -----------------------------------------
 cores/arduino/Print.h   | 14 ++------
 2 files changed, 3 insertions(+), 88 deletions(-)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index 1e4c99a65..bfdcfb99d 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -111,13 +111,6 @@ size_t Print::print(double n, int digits)
   return printFloat(n, digits);
 }
 
-size_t Print::println(const __FlashStringHelper *ifsh)
-{
-  size_t n = print(ifsh);
-  n += println();
-  return n;
-}
-
 size_t Print::print(const Printable& x)
 {
   return x.printTo(*this);
@@ -128,76 +121,6 @@ size_t Print::println(void)
   return write("\r\n");
 }
 
-size_t Print::println(const String &s)
-{
-  size_t n = print(s);
-  n += println();
-  return n;
-}
-
-size_t Print::println(const char c[])
-{
-  size_t n = print(c);
-  n += println();
-  return n;
-}
-
-size_t Print::println(char c)
-{
-  size_t n = print(c);
-  n += println();
-  return n;
-}
-
-size_t Print::println(unsigned char b, int base)
-{
-  size_t n = print(b, base);
-  n += println();
-  return n;
-}
-
-size_t Print::println(int num, int base)
-{
-  size_t n = print(num, base);
-  n += println();
-  return n;
-}
-
-size_t Print::println(unsigned int num, int base)
-{
-  size_t n = print(num, base);
-  n += println();
-  return n;
-}
-
-size_t Print::println(long num, int base)
-{
-  size_t n = print(num, base);
-  n += println();
-  return n;
-}
-
-size_t Print::println(unsigned long num, int base)
-{
-  size_t n = print(num, base);
-  n += println();
-  return n;
-}
-
-size_t Print::println(double num, int digits)
-{
-  size_t n = print(num, digits);
-  n += println();
-  return n;
-}
-
-size_t Print::println(const Printable& x)
-{
-  size_t n = print(x);
-  n += println();
-  return n;
-}
-
 // Private Methods /////////////////////////////////////////////////////////////
 
 size_t Print::printNumber(unsigned long n, uint8_t base)
diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 058a2abbd..c820a6ab4 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -74,20 +74,12 @@ class Print
     size_t print(double, int = 2);
     size_t print(const Printable&);
 
-    size_t println(const __FlashStringHelper *);
-    size_t println(const String &s);
-    size_t println(const char[]);
-    size_t println(char);
-    size_t println(unsigned char, int = DEC);
-    size_t println(int, int = DEC);
-    size_t println(unsigned int, int = DEC);
-    size_t println(long, int = DEC);
-    size_t println(unsigned long, int = DEC);
-    size_t println(double, int = 2);
-    size_t println(const Printable&);
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
+
+    template<typename T> size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
+    template<typename T> size_t println(const T &n, int f) { size_t t = print(n, f); return t + println(); }
 };
 
 #endif

From d9a86828121002fdddd10480c6e14be6500df662 Mon Sep 17 00:00:00 2001
From: cousteaulecommandant <cousteaulecommandant@gmail.com>
Date: Tue, 10 Jan 2017 19:03:49 +0100
Subject: [PATCH 02/19] Inline some print() functions

Remove print(char | char[] | Printable | unsigned char | int | unsigned int)
from Print.cpp and replace them with inline versions in Print.h.
---
 cores/arduino/Print.cpp | 30 ------------------------------
 cores/arduino/Print.h   | 16 +++++++++-------
 2 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index bfdcfb99d..723db4a9b 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -59,31 +59,6 @@ size_t Print::print(const String &s)
   return write(s.c_str(), s.length());
 }
 
-size_t Print::print(const char str[])
-{
-  return write(str);
-}
-
-size_t Print::print(char c)
-{
-  return write(c);
-}
-
-size_t Print::print(unsigned char b, int base)
-{
-  return print((unsigned long) b, base);
-}
-
-size_t Print::print(int n, int base)
-{
-  return print((long) n, base);
-}
-
-size_t Print::print(unsigned int n, int base)
-{
-  return print((unsigned long) n, base);
-}
-
 size_t Print::print(long n, int base)
 {
   if (base == 0) {
@@ -111,11 +86,6 @@ size_t Print::print(double n, int digits)
   return printFloat(n, digits);
 }
 
-size_t Print::print(const Printable& x)
-{
-  return x.printTo(*this);
-}
-
 size_t Print::println(void)
 {
   return write("\r\n");
diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index c820a6ab4..71f91758a 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -64,16 +64,18 @@ class Print
 
     size_t print(const __FlashStringHelper *);
     size_t print(const String &);
-    size_t print(const char[]);
-    size_t print(char);
-    size_t print(unsigned char, int = DEC);
-    size_t print(int, int = DEC);
-    size_t print(unsigned int, int = DEC);
     size_t print(long, int = DEC);
     size_t print(unsigned long, int = DEC);
     size_t print(double, int = 2);
-    size_t print(const Printable&);
-
+    
+    size_t print(const char str[])   { return write(str); }
+    size_t print(const char c)       { return write(c); }
+    size_t print(const Printable &x) { return x.printTo(*this); }
+    
+    size_t print(unsigned char  n, int f = DEC) { return print((unsigned long) n, f); }
+    size_t print(         int   n, int f = DEC) { return print((         long) n, f); }
+    size_t print(unsigned int   n, int f = DEC) { return print((unsigned long) n, f); }
+    
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }

From da0b943248f5258667be27ad3568ff1d2842525a Mon Sep 17 00:00:00 2001
From: cousteaulecommandant <cousteaulecommandant@gmail.com>
Date: Tue, 10 Jan 2017 19:14:37 +0100
Subject: [PATCH 03/19] Add print(signed char | short | float) explicitly

Add explicit print() support for signed char, unsigned/signed short, and float.
Also, add `signed` keyword explicitly to int and long (even if unnecessary).

Notice that `char` and `signed char` are considered different types,
even if their ranges are the same (ditto for `unsigned char` in SAM).
Both AVR and SAM seem to define [u]int8_t explicitly as [un]signed char
and not as char, so printing them will print them as numbers as expected.
(This does not apply to `int` and `signed int`; those are the same type.)
---
 cores/arduino/Print.cpp | 2 +-
 cores/arduino/Print.h   | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index 723db4a9b..3f9534c0e 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -59,7 +59,7 @@ size_t Print::print(const String &s)
   return write(s.c_str(), s.length());
 }
 
-size_t Print::print(long n, int base)
+size_t Print::print(signed long n, int base)
 {
   if (base == 0) {
     return write(n);
diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 71f91758a..3b30d2ce3 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -64,7 +64,7 @@ class Print
 
     size_t print(const __FlashStringHelper *);
     size_t print(const String &);
-    size_t print(long, int = DEC);
+    size_t print(  signed long, int = DEC);
     size_t print(unsigned long, int = DEC);
     size_t print(double, int = 2);
     
@@ -72,9 +72,13 @@ class Print
     size_t print(const char c)       { return write(c); }
     size_t print(const Printable &x) { return x.printTo(*this); }
     
+    size_t print(  signed char  n, int f = DEC) { return print((  signed long) n, f); }
+    size_t print(  signed short n, int f = DEC) { return print((  signed long) n, f); }
+    size_t print(  signed int   n, int f = DEC) { return print((  signed long) n, f); }
     size_t print(unsigned char  n, int f = DEC) { return print((unsigned long) n, f); }
-    size_t print(         int   n, int f = DEC) { return print((         long) n, f); }
+    size_t print(unsigned short n, int f = DEC) { return print((unsigned long) n, f); }
     size_t print(unsigned int   n, int f = DEC) { return print((unsigned long) n, f); }
+    size_t print(    float      n, int f = 2  ) { return print((    double   ) n, f); }
     
     size_t println(void);
 

From a10af6b0c61206bb89e51326ebda3daf873f1b57 Mon Sep 17 00:00:00 2001
From: cousteaulecommandant <cousteaulecommandant@gmail.com>
Date: Tue, 10 Jan 2017 20:00:31 +0100
Subject: [PATCH 04/19] Force inlining of inline print[ln] methods

Add GCC __attribute__ ((__always_inline__)) to inlined methods in Print.h.
This seems necessary since not doing so increases the executable size
(probably because that would create several function definitions),
even with LTO optimizations.
This change combined with the inlining of print/printf methods seems to reduce
the executable size for most applications by a fair amount.
(Note that the `inline` keyword does not imply actual inlining.)
---
 cores/arduino/Print.h | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 3b30d2ce3..ff02b1df8 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -34,6 +34,8 @@
 #endif
 #define BIN 2
 
+#define _always_inline __attribute__ ((__always_inline__)) // undefined at end
+
 class Print
 {
   private:
@@ -68,24 +70,26 @@ class Print
     size_t print(unsigned long, int = DEC);
     size_t print(double, int = 2);
     
-    size_t print(const char str[])   { return write(str); }
-    size_t print(const char c)       { return write(c); }
-    size_t print(const Printable &x) { return x.printTo(*this); }
+    _always_inline size_t print(const char str[])   { return write(str); }
+    _always_inline size_t print(const char c)       { return write(c); }
+    _always_inline size_t print(const Printable &x) { return x.printTo(*this); }
     
-    size_t print(  signed char  n, int f = DEC) { return print((  signed long) n, f); }
-    size_t print(  signed short n, int f = DEC) { return print((  signed long) n, f); }
-    size_t print(  signed int   n, int f = DEC) { return print((  signed long) n, f); }
-    size_t print(unsigned char  n, int f = DEC) { return print((unsigned long) n, f); }
-    size_t print(unsigned short n, int f = DEC) { return print((unsigned long) n, f); }
-    size_t print(unsigned int   n, int f = DEC) { return print((unsigned long) n, f); }
-    size_t print(    float      n, int f = 2  ) { return print((    double   ) n, f); }
+    _always_inline size_t print(  signed char  n, int f = DEC) { return print((  signed long) n, f); }
+    _always_inline size_t print(  signed short n, int f = DEC) { return print((  signed long) n, f); }
+    _always_inline size_t print(  signed int   n, int f = DEC) { return print((  signed long) n, f); }
+    _always_inline size_t print(unsigned char  n, int f = DEC) { return print((unsigned long) n, f); }
+    _always_inline size_t print(unsigned short n, int f = DEC) { return print((unsigned long) n, f); }
+    _always_inline size_t print(unsigned int   n, int f = DEC) { return print((unsigned long) n, f); }
+    _always_inline size_t print(    float      n, int f = 2  ) { return print((    double   ) n, f); }
     
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
 
-    template<typename T> size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
-    template<typename T> size_t println(const T &n, int f) { size_t t = print(n, f); return t + println(); }
+    template<typename T> _always_inline size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
+    template<typename T> _always_inline size_t println(const T &n, int f) { size_t t = print(n, f); return t + println(); }
 };
 
+#undef _always_inline
+
 #endif

From a79b4450c7f21940a99203b10164aff5ea6676d6 Mon Sep 17 00:00:00 2001
From: cousteaulecommandant <cousteaulecommandant@gmail.com>
Date: Tue, 10 Jan 2017 20:31:07 +0100
Subject: [PATCH 05/19] Add support for variadic Serial.print(...)

Allows combining multiple arguments into a single call to Print::print[ln], like Serial.print("Answer=", 66, HEX).
This feature requires C++11 or newer to work, but if it is not available the old calls can still be used (the change has been wrapped in `#if __cplusplus >= 201103L ... #endif`).
This change is backwards compatible: print(42, 16) prints "2A" as always, not "4216" (the latter can still be achieved with print(42, "", 16) ).  This also works with multiple arguments: any numeric argument followed by an int will be treated as a (number, format) pair.
The variadic function calls are inlined using an __attribute__ ((__always_inline__)), so they're equivalent to writing multiple sequential calls to print with one argument.
---
 cores/arduino/Print.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index ff02b1df8..9b44387ef 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -86,8 +86,38 @@ class Print
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
 
+#if __cplusplus >= 201103L
+    template<typename ...Ts> _always_inline size_t println(const Ts &...args) { size_t t = print(args...); return t + println(); }
+#else
     template<typename T> _always_inline size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
     template<typename T> _always_inline size_t println(const T &n, int f) { size_t t = print(n, f); return t + println(); }
+#endif // __cplusplus >= 201103L
+
+
+    /** Variadic methods **/
+#if __cplusplus >= 201103L  // requires C++11
+    // Ensure there are at least two parameters to avoid infinite recursion.
+    // e.g. `StringSumHelper s; print(s)` may be treated as `print(s, ...)` 
+    // with `...` being the empty list, thus calling `print(s)` again.
+    // (This is because print(StringSumHelper) isn't explicitly defined.)
+    template<typename T, typename T2, typename ...Ts>
+    _always_inline size_t print(const T &arg, const T2 &arg2, const Ts &...args) {
+      size_t t = print(arg);
+      return t + print(arg2, args...);
+    }
+    // Some methods take an extra int parameter.  If so, use these templates.
+    // In a future, it would be nice to make the base/precision a special type.
+    template<typename ...Ts> _always_inline size_t print(  signed char  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(  signed short n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(  signed int   n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(  signed long  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(unsigned char  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(unsigned short n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(unsigned int   n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(unsigned long  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(    float      n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+    template<typename ...Ts> _always_inline size_t print(    double     n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+#endif // __cplusplus >= 201103L
 };
 
 #undef _always_inline

From 5084817494bbed7301735b40bb864a98f1d6af3d Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 19 Jun 2017 17:45:46 +0200
Subject: [PATCH 06/19] WIP: Custom formatters

This allows passing formatters to print. E.g. instead of print(10, HEX),
you can pass any object that has a suitable printTo(Print*, ValueT)
method.

This is a WIP on top of the variadic print PR and it makes some changes
that should probably go into there.

As an example of usage:

	struct EvenOddFormatter {
	  int printTo(Print *p, int x) {
	    return p->print(x % 2 ? "odd" : "even");
	  }
	};

	Serial.println(1, EvenOddFormatter());
---
 cores/arduino/Print.cpp | 10 +++---
 cores/arduino/Print.h   | 70 +++++++++++++++++++++++++++--------------
 2 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index 3f9534c0e..13c2c5604 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -41,7 +41,7 @@ size_t Print::write(const uint8_t *buffer, size_t size)
   return n;
 }
 
-size_t Print::print(const __FlashStringHelper *ifsh)
+size_t Print::doPrint(const __FlashStringHelper *ifsh)
 {
   PGM_P p = reinterpret_cast<PGM_P>(ifsh);
   size_t n = 0;
@@ -54,12 +54,12 @@ size_t Print::print(const __FlashStringHelper *ifsh)
   return n;
 }
 
-size_t Print::print(const String &s)
+size_t Print::doPrint(const String &s)
 {
   return write(s.c_str(), s.length());
 }
 
-size_t Print::print(signed long n, int base)
+size_t Print::doPrint(signed long n, int base)
 {
   if (base == 0) {
     return write(n);
@@ -75,13 +75,13 @@ size_t Print::print(signed long n, int base)
   }
 }
 
-size_t Print::print(unsigned long n, int base)
+size_t Print::doPrint(unsigned long n, int base)
 {
   if (base == 0) return write(n);
   else return printNumber(n, base);
 }
 
-size_t Print::print(double n, int digits)
+size_t Print::doPrint(double n, int digits)
 {
   return printFloat(n, digits);
 }
diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 9b44387ef..03023996c 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -60,28 +60,41 @@ class Print
       return write((const uint8_t *)buffer, size);
     }
 
+
     // default to zero, meaning "a single write may block"
     // should be overriden by subclasses with buffering
     virtual int availableForWrite() { return 0; }
 
-    size_t print(const __FlashStringHelper *);
-    size_t print(const String &);
-    size_t print(  signed long, int = DEC);
-    size_t print(unsigned long, int = DEC);
-    size_t print(double, int = 2);
-    
-    _always_inline size_t print(const char str[])   { return write(str); }
-    _always_inline size_t print(const char c)       { return write(c); }
-    _always_inline size_t print(const Printable &x) { return x.printTo(*this); }
+    size_t doPrint(const __FlashStringHelper *);
+    size_t doPrint(const String &);
+    size_t doPrint(  signed long, int = DEC);
+    size_t doPrint(unsigned long, int = DEC);
+    size_t doPrint(double, int = 2);
     
-    _always_inline size_t print(  signed char  n, int f = DEC) { return print((  signed long) n, f); }
-    _always_inline size_t print(  signed short n, int f = DEC) { return print((  signed long) n, f); }
-    _always_inline size_t print(  signed int   n, int f = DEC) { return print((  signed long) n, f); }
-    _always_inline size_t print(unsigned char  n, int f = DEC) { return print((unsigned long) n, f); }
-    _always_inline size_t print(unsigned short n, int f = DEC) { return print((unsigned long) n, f); }
-    _always_inline size_t print(unsigned int   n, int f = DEC) { return print((unsigned long) n, f); }
-    _always_inline size_t print(    float      n, int f = 2  ) { return print((    double   ) n, f); }
+    _always_inline size_t doPrint(const char str[])   { return write(str); }
+    _always_inline size_t doPrint(const char c)       { return write(c); }
+    _always_inline size_t doPrint(const Printable &x) { return x.printTo(*this); }
     
+    _always_inline size_t doPrint(  signed char  n, int f = DEC) { return doPrint((  signed long) n, f); }
+    _always_inline size_t doPrint(  signed short n, int f = DEC) { return doPrint((  signed long) n, f); }
+    _always_inline size_t doPrint(  signed int   n, int f = DEC) { return doPrint((  signed long) n, f); }
+    _always_inline size_t doPrint(unsigned char  n, int f = DEC) { return doPrint((unsigned long) n, f); }
+    _always_inline size_t doPrint(unsigned short n, int f = DEC) { return doPrint((unsigned long) n, f); }
+    _always_inline size_t doPrint(unsigned int   n, int f = DEC) { return doPrint((unsigned long) n, f); }
+    _always_inline size_t doPrint(    float      n, int f = 2  ) { return doPrint((    double   ) n, f); }
+
+    template <typename Check, typename T>
+    struct check_type {
+      using type = T;
+    };
+    template <typename Check, typename T> using check_type_t = typename check_type<Check, T>::type;
+
+    template<typename T, typename F>
+    _always_inline auto doPrint(T v, F f )
+    -> check_type_t<decltype(f.printTo(this, v)), size_t> {
+      return f.printTo(this, v);
+    }
+ 
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
@@ -90,21 +103,26 @@ class Print
     template<typename ...Ts> _always_inline size_t println(const Ts &...args) { size_t t = print(args...); return t + println(); }
 #else
     template<typename T> _always_inline size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
-    template<typename T> _always_inline size_t println(const T &n, int f) { size_t t = print(n, f); return t + println(); }
+    template<typename T, typename T2> _always_inline size_t println(const T &arg1, const T2& arg2) { size_t t = print(arg1, arg2); return t + println(); }
 #endif // __cplusplus >= 201103L
 
+    _always_inline size_t print() { return 0; }
 
     /** Variadic methods **/
 #if __cplusplus >= 201103L  // requires C++11
-    // Ensure there are at least two parameters to avoid infinite recursion.
-    // e.g. `StringSumHelper s; print(s)` may be treated as `print(s, ...)` 
-    // with `...` being the empty list, thus calling `print(s)` again.
-    // (This is because print(StringSumHelper) isn't explicitly defined.)
+    template<typename T, typename ...Ts>
+    _always_inline size_t print(const T &arg, const Ts &...args) {
+      size_t t = doPrint(arg);
+      return t + print(args...);
+    }
+
     template<typename T, typename T2, typename ...Ts>
-    _always_inline size_t print(const T &arg, const T2 &arg2, const Ts &...args) {
-      size_t t = print(arg);
-      return t + print(arg2, args...);
+    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
+    -> check_type_t<decltype(doPrint(arg, arg2)), size_t> {
+      size_t t = doPrint(arg, arg2);
+      return t + print(args...);
     }
+/*
     // Some methods take an extra int parameter.  If so, use these templates.
     // In a future, it would be nice to make the base/precision a special type.
     template<typename ...Ts> _always_inline size_t print(  signed char  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
@@ -117,6 +135,10 @@ class Print
     template<typename ...Ts> _always_inline size_t print(unsigned long  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
     template<typename ...Ts> _always_inline size_t print(    float      n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
     template<typename ...Ts> _always_inline size_t print(    double     n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
+*/
+#else
+    template<typename T> _always_inline size_t print(const T &arg)      { return doPrint(arg); }
+    template<typename T, typename T2> _always_inline size_t print(const T &arg1, const T2& arg2) { return doPrint(arg1, arg2); }
 #endif // __cplusplus >= 201103L
 };
 

From c64a4e03948d6217fbc360f2c6b9e481943c4635 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Wed, 30 May 2018 00:09:45 +0200
Subject: [PATCH 07/19] WIP

---
 cores/arduino/Print.h | 210 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 203 insertions(+), 7 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 03023996c..3877c25c0 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -36,6 +36,12 @@
 
 #define _always_inline __attribute__ ((__always_inline__)) // undefined at end
 
+template <typename Check, typename T>
+struct check_type {
+  using type = T;
+};
+template <typename Check, typename T> using check_type_t = typename check_type<Check, T>::type;
+
 class Print
 {
   private:
@@ -47,6 +53,18 @@ class Print
   public:
     Print() : write_error(0) {}
   
+    struct Formatter { };
+
+    struct FormatterOptionBase { };
+    template <typename TFormatter, typename TValue>
+    struct FormatterOption : FormatterOptionBase {
+      // Is this the right place to define this? Or perhaps just move
+      // down?
+      using Formatter = TFormatter;
+      TValue value;
+      constexpr FormatterOption(const TValue value) : value(value) { }
+    };
+
     int getWriteError() { return write_error; }
     void clearWriteError() { setWriteError(0); }
   
@@ -83,12 +101,6 @@ class Print
     _always_inline size_t doPrint(unsigned int   n, int f = DEC) { return doPrint((unsigned long) n, f); }
     _always_inline size_t doPrint(    float      n, int f = 2  ) { return doPrint((    double   ) n, f); }
 
-    template <typename Check, typename T>
-    struct check_type {
-      using type = T;
-    };
-    template <typename Check, typename T> using check_type_t = typename check_type<Check, T>::type;
-
     template<typename T, typename F>
     _always_inline auto doPrint(T v, F f )
     -> check_type_t<decltype(f.printTo(this, v)), size_t> {
@@ -98,18 +110,48 @@ class Print
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
+    /*
+    template<typename T, typename ...Ts>
+    _always_inline size_t print(const T &arg, const Ts &...args) {
+      // This might lead to infinite template instantion
+      //return print(arg, DefaultFormatter<>(), args...);
+      size_t n = DefaultFormatter<>().printTo(this, arg);
+      return n + print(args...);
+    }
+
+    template<typename T, typename T2, typename ...Ts>
+    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
+    -> check_type_t<decltype(arg2.printTo(this, arg)), size_t> {
+      size_t n = arg2.printTo(this, arg);
+      return n + print(args...);
+    }
+
+    template<typename T, typename T2, typename T3, typename ...Ts>
+    _always_inline auto print(const T &arg, const T2 &arg2, const T3 &arg3, const Ts &...args)
+    -> check_type_t<decltype(arg2.addOption(arg3)), size_t> {
+      return print(arg, arg2.addOption(arg3), args...);
+    }
+
+    template<typename T, typename T2, typename ...Ts>
+    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
+    -> check_type_t<decltype(DefaultFormatter<>().addOption(arg2)), size_t> {
+      return print(arg, DefaultFormatter<>().addOption(arg2), args...);
+    }
+    */
 
 #if __cplusplus >= 201103L
+    template<typename ...Ts> _always_inline size_t print(const Ts &...args);
     template<typename ...Ts> _always_inline size_t println(const Ts &...args) { size_t t = print(args...); return t + println(); }
 #else
     template<typename T> _always_inline size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
     template<typename T, typename T2> _always_inline size_t println(const T &arg1, const T2& arg2) { size_t t = print(arg1, arg2); return t + println(); }
 #endif // __cplusplus >= 201103L
 
-    _always_inline size_t print() { return 0; }
+    //_always_inline size_t print() { return 0; }
 
     /** Variadic methods **/
 #if __cplusplus >= 201103L  // requires C++11
+/*
     template<typename T, typename ...Ts>
     _always_inline size_t print(const T &arg, const Ts &...args) {
       size_t t = doPrint(arg);
@@ -122,6 +164,8 @@ class Print
       size_t t = doPrint(arg, arg2);
       return t + print(args...);
     }
+*/
+
 /*
     // Some methods take an extra int parameter.  If so, use these templates.
     // In a future, it would be nice to make the base/precision a special type.
@@ -142,6 +186,158 @@ class Print
 #endif // __cplusplus >= 201103L
 };
 
+class DefaultFormatter;
+
+// TODO: Do we really need a FormatterOption base class? Without it,
+// options can be POD, not needing a constructor. With it, we can
+// prevent accidentally treating things as options which are not, but if
+// we already check a Formatter base class, we can also require that
+// Formatters do not define nonsensical addOption methods (even more,
+// without an explicit FormatterOption, Formatters can even use
+// non-class types as options if they want).
+// TODO: Where to define the "default formatter" for an option? Now, it
+// is our superclass, but it might just as well be defined with a using
+// directive directly here. Or perhaps it should be a method (this
+// might be problematic, since the DefaultFormatter type is incomplete
+// at this point). One completely different approach would be a
+// DefaultFormatterFor template, which gets specialized, but this
+// probably has the problem that once it is instantiated, you can no
+// longer add to it. Using specializations does allow defining a default
+// formatter for a type/option combination (allowing reuse of e.g. HEX
+// for custom types) or for just a type (allowing default formatting of
+// custom types).
+struct FormatOptionBase : Print::FormatterOption<DefaultFormatter, uint8_t> {
+  // TODO: We must provide an explicit constructor (if we have a
+  // superclass, we are no longer a POD type), and if we do, there is no
+  // real point in storing our value in the superclass (better have one
+  // extra line here, but more explictness. Inheriting the constructor
+  // is even more ugly (needs to repeat superclass template arguments).
+  constexpr FormatOptionBase(uint8_t value) : Print::FormatterOption<DefaultFormatter, uint8_t>(value) { }
+};
+struct FormatOptionMinWidth : Print::FormatterOption<DefaultFormatter, uint8_t> {
+  constexpr FormatOptionMinWidth(uint8_t value) : Print::FormatterOption<DefaultFormatter, uint8_t>(value) { }
+};
+
+#undef HEX
+inline constexpr FormatOptionMinWidth PRINT_MIN_WIDTH(uint8_t min_width) { return {min_width}; }
+inline constexpr FormatOptionBase PRINT_BASE(uint8_t base) { return {base}; }
+constexpr FormatOptionBase HEX = PRINT_BASE(16);
+
+struct DefaultFormatter : Print::Formatter {
+  uint8_t base;
+  uint8_t min_width;
+
+  DefaultFormatter(uint8_t base = 10, uint8_t min_width = 0)
+    : base(base), min_width(min_width)
+  { }
+
+  DefaultFormatter addOption(FormatOptionBase o) const {
+    return {o.value, this->min_width};
+  }
+
+  DefaultFormatter addOption(FormatOptionMinWidth o) const {
+    return {this->base, o.value};
+  }
+
+  size_t printTo(Print *p, int n) const;
+  size_t printTo(Print *p, const char *) const;
+  template <typename T>
+  size_t printTo(Print *p, const T&) const;
+};
+
+// TODO: These can probably be moved back inline above (they were split
+// when PrintHelper did not exist, so the Print class was defined
+// between the declaration of DefaultFormatter and these method
+// definitions).
+inline size_t DefaultFormatter::printTo(Print *p, int n) const {
+  return p->doPrint(n, this->base);
+}
+
+inline size_t DefaultFormatter::printTo(Print *p, const char * s) const {
+  // TODO: Maybe replace bool has_int_options with a more detailed bitwise flag int
+  // for better error messages.
+  //static_assert(!has_int_options, "Cannot print strings with integer-specific formatting options");
+  p->doPrint(s);
+  return 0;
+}
+
+template <typename T>
+inline size_t DefaultFormatter::printTo(Print *p, const T& v) const {
+  return p->doPrint(v);
+}
+
+// TODO: Should we really need PrintHelper? It was now added allow
+// referencing DefaultFormatter above. These methods could just be
+// out-of-line definitions of methods in Print, but then the complicated
+// method signature must be duplicated. Alternatively, a single method
+// that generates a DefaultFormatter object could possibly be out of
+// line (though, thinking on it, both of these options might not work,
+// since they need DefaultFormatter as a return type in a
+// declaration...).
+class PrintHelper {
+  public:
+    template<typename T, typename ...Ts>
+    static _always_inline size_t printTo(Print * p, const T &arg, const Ts &...args) {
+      // This might lead to infinite template instantion
+      //return print(arg, DefaultFormatter<>(), args...);
+      size_t n = DefaultFormatter().printTo(p, arg);
+      return n + printTo(p, args...);
+    }
+/*
+    template<typename T, typename T2, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
+    -> check_type_t<decltype(arg2.printTo(p, arg)), size_t> {
+      size_t n = arg2.printTo(p, arg);
+      return n + printTo(p, args...);
+    }
+*/
+    static void accepts_formatter(const Print::Formatter*);
+
+    template<typename T, typename T2, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
+    -> check_type_t<decltype(accepts_formatter(&arg2)), size_t> {
+    //-> check_type_t<decltype(arg2.printTo(p, arg)), size_t> {
+      size_t n = arg2.printTo(p, arg);
+      return n + printTo(p, args...);
+    }
+
+
+    template<typename T, typename T2, typename T3, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const T3 &arg3, const Ts &...args)
+    -> check_type_t<decltype(arg2.addOption(arg3)), size_t> {
+      return printTo(p, arg, arg2.addOption(arg3), args...);
+    }
+
+    static void accepts_option(const Print::FormatterOptionBase*);
+
+    template<typename T, typename T2, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
+    //-> check_type_t<decltype(typename T2::Formatter().addOption(arg2)), size_t> {
+    -> check_type_t<decltype(accepts_option(&arg2)), size_t> {
+      //accepts_option(&arg2);
+      return printTo(p, arg, typename T2::Formatter().addOption(arg2), args...);
+    }
+
+    static _always_inline size_t printTo(Print *) { return 0; }
+};
+
+template<typename ...Ts>
+_always_inline size_t Print::print(const Ts &...args) { return PrintHelper::printTo(this, args...); }
 #undef _always_inline
 
 #endif
+
+// Idea: Raise errors when DefaultFormatter is used with int-only
+// options (e.g. integer base), but prints a string or otherwise
+// incompatible type. This can be done by extending DefaultFormatter
+// with a bitmask or bool template argument(s) that track what options
+// have been set, so they can be static_asserted against in specific
+// printTo versions. This does require that *all* methods in
+// DefaultFormatter are always_inline, to prevent duplicates. This means
+// the actual printing code must again live elsewhere, which might not
+// be ideal. Also, error messages are then complicated with these
+// template arguments. This can be solved by doing just the checks and
+// forwarding *all* printTo calls to another class (including a
+// catch-all template), so that when you try printing any unsupported
+// types you still get the proper "no such method to call, candiates
+// are..." error message.

From fa23c187ab4f4ce5817ed824f82407e531a4a903 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Fri, 1 Jun 2018 18:00:09 +0200
Subject: [PATCH 08/19] WIP

---
 cores/arduino/Print.cpp |  75 +++----
 cores/arduino/Print.h   | 482 ++++++++++++++++++++++++++++++----------
 2 files changed, 392 insertions(+), 165 deletions(-)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index 13c2c5604..00c1b12b7 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -41,66 +41,55 @@ size_t Print::write(const uint8_t *buffer, size_t size)
   return n;
 }
 
-size_t Print::doPrint(const __FlashStringHelper *ifsh)
+size_t Print::println(void)
 {
-  PGM_P p = reinterpret_cast<PGM_P>(ifsh);
+  return write("\r\n");
+}
+
+
+size_t Formatters::DefaultFormatter::printTo(Print *p, const __FlashStringHelper *ifsh) const
+{
+  PGM_P ptr = reinterpret_cast<PGM_P>(ifsh);
   size_t n = 0;
   while (1) {
-    unsigned char c = pgm_read_byte(p++);
+    unsigned char c = pgm_read_byte(ptr++);
     if (c == 0) break;
-    if (write(c)) n++;
+    if (p->write(c)) n++;
     else break;
   }
   return n;
 }
 
-size_t Print::doPrint(const String &s)
+size_t Formatters::DefaultFormatter::printTo(Print *p, const String &s) const
 {
-  return write(s.c_str(), s.length());
+  return p->write(s.c_str(), s.length());
 }
 
-size_t Print::doPrint(signed long n, int base)
+size_t Formatters::DefaultFormatter::printSignedNumber(Print *p, signed long n) const
 {
-  if (base == 0) {
-    return write(n);
-  } else if (base == 10) {
+  if (this->base == 10) {
     if (n < 0) {
-      int t = print('-');
+      size_t t = p->write('-');
       n = -n;
-      return printNumber(n, 10) + t;
+      return printNumber(p, n) + t;
     }
-    return printNumber(n, 10);
+    return printNumber(p, n);
   } else {
-    return printNumber(n, base);
+    return printNumber(p, n);
   }
 }
 
-size_t Print::doPrint(unsigned long n, int base)
-{
-  if (base == 0) return write(n);
-  else return printNumber(n, base);
-}
-
-size_t Print::doPrint(double n, int digits)
-{
-  return printFloat(n, digits);
-}
-
-size_t Print::println(void)
-{
-  return write("\r\n");
-}
-
 // Private Methods /////////////////////////////////////////////////////////////
 
-size_t Print::printNumber(unsigned long n, uint8_t base)
+size_t Formatters::DefaultFormatter::printNumber(Print *p , unsigned long n) const
 {
   char buf[8 * sizeof(long) + 1]; // Assumes 8-bit chars plus zero byte.
   char *str = &buf[sizeof(buf) - 1];
+  uint8_t base = this->base;
 
   *str = '\0';
 
-  // prevent crash if called with base == 1
+  // prevent crash if called with base 0 or 1
   if (base < 2) base = 10;
 
   do {
@@ -110,22 +99,24 @@ size_t Print::printNumber(unsigned long n, uint8_t base)
     *--str = c < 10 ? c + '0' : c + 'A' - 10;
   } while(n);
 
-  return write(str);
+  return p->write(str);
 }
 
-size_t Print::printFloat(double number, uint8_t digits) 
+size_t Formatters::DefaultFormatter::printFloat(Print *p, double number) const
 { 
   size_t n = 0;
+  uint8_t digits = this->precision;
+  auto int_formatter = DefaultFormatter();
   
-  if (isnan(number)) return print("nan");
-  if (isinf(number)) return print("inf");
-  if (number > 4294967040.0) return print ("ovf");  // constant determined empirically
-  if (number <-4294967040.0) return print ("ovf");  // constant determined empirically
+  if (isnan(number)) return p->write("nan");
+  if (isinf(number)) return p->write("inf");
+  if (number > 4294967040.0) return p->write ("ovf");  // constant determined empirically
+  if (number <-4294967040.0) return p->write ("ovf");  // constant determined empirically
   
   // Handle negative numbers
   if (number < 0.0)
   {
-     n += print('-');
+     n += p->write('-');
      number = -number;
   }
 
@@ -139,11 +130,11 @@ size_t Print::printFloat(double number, uint8_t digits)
   // Extract the integer part of the number and print it
   unsigned long int_part = (unsigned long)number;
   double remainder = number - (double)int_part;
-  n += print(int_part);
+  n += int_formatter.printTo(p, int_part);
 
   // Print the decimal point, but only if there are digits beyond
   if (digits > 0) {
-    n += print('.'); 
+    n += p->write('.');
   }
 
   // Extract digits from the remainder one at a time
@@ -151,7 +142,7 @@ size_t Print::printFloat(double number, uint8_t digits)
   {
     remainder *= 10.0;
     unsigned int toPrint = (unsigned int)(remainder);
-    n += print(toPrint);
+    n += int_formatter.printTo(p, toPrint);
     remainder -= toPrint; 
   } 
   
diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 3877c25c0..444327469 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -37,33 +37,24 @@
 #define _always_inline __attribute__ ((__always_inline__)) // undefined at end
 
 template <typename Check, typename T>
-struct check_type {
+struct enable_if_valid {
   using type = T;
 };
-template <typename Check, typename T> using check_type_t = typename check_type<Check, T>::type;
+template <typename Check, typename T> using enable_if_valid_t = typename enable_if_valid<Check, T>::type;
 
 class Print
 {
   private:
     int write_error;
-    size_t printNumber(unsigned long, uint8_t);
-    size_t printFloat(double, uint8_t);
   protected:
     void setWriteError(int err = 1) { write_error = err; }
   public:
     Print() : write_error(0) {}
   
+    // TODO: Move these into the Formatters namespace?
+    // TODO: Shorten / rethink these names?
     struct Formatter { };
-
-    struct FormatterOptionBase { };
-    template <typename TFormatter, typename TValue>
-    struct FormatterOption : FormatterOptionBase {
-      // Is this the right place to define this? Or perhaps just move
-      // down?
-      using Formatter = TFormatter;
-      TValue value;
-      constexpr FormatterOption(const TValue value) : value(value) { }
-    };
+    struct FormatterOption { };
 
     int getWriteError() { return write_error; }
     void clearWriteError() { setWriteError(0); }
@@ -83,29 +74,6 @@ class Print
     // should be overriden by subclasses with buffering
     virtual int availableForWrite() { return 0; }
 
-    size_t doPrint(const __FlashStringHelper *);
-    size_t doPrint(const String &);
-    size_t doPrint(  signed long, int = DEC);
-    size_t doPrint(unsigned long, int = DEC);
-    size_t doPrint(double, int = 2);
-    
-    _always_inline size_t doPrint(const char str[])   { return write(str); }
-    _always_inline size_t doPrint(const char c)       { return write(c); }
-    _always_inline size_t doPrint(const Printable &x) { return x.printTo(*this); }
-    
-    _always_inline size_t doPrint(  signed char  n, int f = DEC) { return doPrint((  signed long) n, f); }
-    _always_inline size_t doPrint(  signed short n, int f = DEC) { return doPrint((  signed long) n, f); }
-    _always_inline size_t doPrint(  signed int   n, int f = DEC) { return doPrint((  signed long) n, f); }
-    _always_inline size_t doPrint(unsigned char  n, int f = DEC) { return doPrint((unsigned long) n, f); }
-    _always_inline size_t doPrint(unsigned short n, int f = DEC) { return doPrint((unsigned long) n, f); }
-    _always_inline size_t doPrint(unsigned int   n, int f = DEC) { return doPrint((unsigned long) n, f); }
-    _always_inline size_t doPrint(    float      n, int f = 2  ) { return doPrint((    double   ) n, f); }
-
-    template<typename T, typename F>
-    _always_inline auto doPrint(T v, F f )
-    -> check_type_t<decltype(f.printTo(this, v)), size_t> {
-      return f.printTo(this, v);
-    }
  
     size_t println(void);
 
@@ -121,20 +89,20 @@ class Print
 
     template<typename T, typename T2, typename ...Ts>
     _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> check_type_t<decltype(arg2.printTo(this, arg)), size_t> {
+    -> enable_if_valid_t<decltype(arg2.printTo(this, arg)), size_t> {
       size_t n = arg2.printTo(this, arg);
       return n + print(args...);
     }
 
     template<typename T, typename T2, typename T3, typename ...Ts>
     _always_inline auto print(const T &arg, const T2 &arg2, const T3 &arg3, const Ts &...args)
-    -> check_type_t<decltype(arg2.addOption(arg3)), size_t> {
+    -> enable_if_valid_t<decltype(arg2.addOption(arg3)), size_t> {
       return print(arg, arg2.addOption(arg3), args...);
     }
 
     template<typename T, typename T2, typename ...Ts>
     _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> check_type_t<decltype(DefaultFormatter<>().addOption(arg2)), size_t> {
+    -> enable_if_valid_t<decltype(DefaultFormatter<>().addOption(arg2)), size_t> {
       return print(arg, DefaultFormatter<>().addOption(arg2), args...);
     }
     */
@@ -160,34 +128,38 @@ class Print
 
     template<typename T, typename T2, typename ...Ts>
     _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> check_type_t<decltype(doPrint(arg, arg2)), size_t> {
+    -> enable_if_valid_t<decltype(doPrint(arg, arg2)), size_t> {
       size_t t = doPrint(arg, arg2);
       return t + print(args...);
     }
 */
 
-/*
-    // Some methods take an extra int parameter.  If so, use these templates.
-    // In a future, it would be nice to make the base/precision a special type.
-    template<typename ...Ts> _always_inline size_t print(  signed char  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(  signed short n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(  signed int   n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(  signed long  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(unsigned char  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(unsigned short n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(unsigned int   n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(unsigned long  n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(    float      n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-    template<typename ...Ts> _always_inline size_t print(    double     n, int f, const Ts &...args) { size_t t = print(n, f); return t + print(args...); }
-*/
+    // Compatibility versions of print that take a second base or
+    // precision argument that is an integer.
+    _always_inline size_t print(  signed char , int);
+    _always_inline size_t print(  signed short, int);
+    _always_inline size_t print(  signed int  , int);
+    _always_inline size_t print(  signed long , int);
+    _always_inline size_t print(unsigned char , int);
+    _always_inline size_t print(unsigned short, int);
+    _always_inline size_t print(unsigned int  , int);
+    _always_inline size_t print(unsigned long , int);
+    _always_inline size_t print(    float     , int);
+    _always_inline size_t print(    double    , int);
 #else
     template<typename T> _always_inline size_t print(const T &arg)      { return doPrint(arg); }
     template<typename T, typename T2> _always_inline size_t print(const T &arg1, const T2& arg2) { return doPrint(arg1, arg2); }
 #endif // __cplusplus >= 201103L
 };
 
-class DefaultFormatter;
+void accepts_formatter(const Print::Formatter*);
+void accepts_option(const Print::FormatterOption*);
 
+// Namespace for more advanced custom formatter stuff, to prevent
+// cluttering the global namespace. Could be removed if needed.
+namespace Formatters {
+
+  
 // TODO: Do we really need a FormatterOption base class? Without it,
 // options can be POD, not needing a constructor. With it, we can
 // prevent accidentally treating things as options which are not, but if
@@ -206,65 +178,270 @@ class DefaultFormatter;
 // formatter for a type/option combination (allowing reuse of e.g. HEX
 // for custom types) or for just a type (allowing default formatting of
 // custom types).
-struct FormatOptionBase : Print::FormatterOption<DefaultFormatter, uint8_t> {
-  // TODO: We must provide an explicit constructor (if we have a
-  // superclass, we are no longer a POD type), and if we do, there is no
-  // real point in storing our value in the superclass (better have one
-  // extra line here, but more explictness. Inheriting the constructor
-  // is even more ugly (needs to repeat superclass template arguments).
-  constexpr FormatOptionBase(uint8_t value) : Print::FormatterOption<DefaultFormatter, uint8_t>(value) { }
+
+class DefaultFormatter : public Print::Formatter {
+  public:
+    // Common base class for our options
+    struct FormatterOption : Print::FormatterOption { };
+
+    // TODO: Defining option types and functions is a bit verbose, it
+    // would be nice if there was less boilerplate. Also, it would be
+    // nice if these types would be hidden and only a constructor
+    // function would be visible.
+    // To reduce verbosity:
+    //  - Define a template superclass that allocates storage for a
+    //    value with a templated type. Downside is that inheriting
+    //    constructors is very verbose (needs repeating template
+    //    parameters).
+    //  - Define a template helper class that allocates storage, but also
+    //    has a tag template parameter to make it unique (so you do not
+    //    need to subclass, but can use a type alias to it instead).
+    //    This is still a bit verbose because you need to define a tag
+    //    and the alias.
+    /*
+          template <typename tag, typename TValue>
+          struct FormatterOptionHelper : public FormatterOption  {
+              TValue value;
+              constexpr FormatterOptionHelper(TValue value) : value(value) { }
+          };
+
+          struct FormatOptionPrecisionTag;
+          using FormatOptionPrecision = FormatterOptionHelper<FormatOptionPrecisionTag, uint8_t>;
+    */
+    // Both options might be slightly less verbose, but also more
+    // complex, so might not be worth the trouble.
+    //
+    // To reduce visibility:
+    //  - Make the option classes private, and make the global
+    //    builder functions friend. Downside is an extra friend
+    //    declaration.
+    //    It seems that using a friend *definition* (instead of a
+    //    declaration) is appropriate, which defines a global function
+    //    and makes it a friend at the same time. However, it seems this
+    //    creates a hidden function, only accessible through ADL, which
+    //    does not apply to an argumentless function.
+    //    A normal friend declaration can also be problematic, since it
+    //    forwards declares the function in the same namespace as the
+    //    enclosing class. If that is not where it should end up, the
+    //    builder function should be forward declared before the
+    //    formatter class. But since it references the formatter class,
+    //    this gets complex with forward declarations. This might be ok
+    //    if the formatter class is in the global namespace.
+    //  - Make the option classes private, add a public static builder
+    //    method and replace the global builder function by a constexpr
+    //    function pointer. Downside is an extra variable, which *might*
+    //    end up in the final binary.
+
+    struct FormatOptionBase : FormatterOption {
+      constexpr FormatOptionBase(uint8_t value) : value(value) { }
+      uint8_t value;
+    };
+    constexpr DefaultFormatter operator+(FormatOptionBase o) {
+      return {o.value, this->min_width, this->precision};
+    }
+
+    struct FormatOptionPrecision : FormatterOption {
+      constexpr FormatOptionPrecision(uint8_t value) : value(value) { }
+      uint8_t value;
+    };
+    constexpr DefaultFormatter operator+(FormatOptionPrecision o) {
+      return {this->base, o.value, this->precision};
+    }
+
+    struct FormatOptionMinWidth : FormatterOption {
+      constexpr FormatOptionMinWidth(uint8_t value) : value(value) { }
+      uint8_t value;
+    };
+    constexpr DefaultFormatter operator+(FormatOptionMinWidth o) {
+      return {this->base, this->min_width, o.value};
+    }
+
+    constexpr DefaultFormatter(uint8_t base = 10, uint8_t min_width = 0, uint8_t precision = 2)
+      : base(base), min_width(min_width), precision(precision)
+    { }
+
+    //static constexpr FormatOptionPrecision FORMAT_PRECISION(uint8_t prec) { return {prec}; }
+
+    /* String printing, defined in cpp file */
+    size_t printTo(Print*, const __FlashStringHelper *) const;
+    size_t printTo(Print*, const String &) const;
+
+    /* Stuff that is easy to print */
+    _always_inline size_t printTo(Print* p, const char      str[]) const { return p->write(str); }
+    _always_inline size_t printTo(Print* p, const char      c    ) const { return p->write(c); }
+    _always_inline size_t printTo(Print* p, const Printable &x   ) const { return x.printTo(*p); }
+
+    /* Integer printing, upcast to (unsigned) long and then printed usign
+     * a shared print(Signed)Number function. */
+    _always_inline size_t printTo(Print* p,   signed char  n) const { return printSignedNumber(p, n); }
+    _always_inline size_t printTo(Print* p,   signed short n) const { return printSignedNumber(p, n); }
+    _always_inline size_t printTo(Print* p,   signed int   n) const { return printSignedNumber(p, n); }
+    _always_inline size_t printTo(Print* p,   signed long  n) const { return printSignedNumber(p, n); }
+    _always_inline size_t printTo(Print* p, unsigned char  n) const { return printNumber(p, n); }
+    _always_inline size_t printTo(Print* p, unsigned short n) const { return printNumber(p, n); }
+    _always_inline size_t printTo(Print* p, unsigned int   n) const { return printNumber(p, n); }
+    _always_inline size_t printTo(Print* p, unsigned long  n) const { return printNumber(p, n); }
+
+    /* Float is converted to double first */
+    _always_inline size_t printTo(Print* p,     float      n) const { return printFloat(p, n); }
+    _always_inline size_t printTo(Print* p,     double     n) const { return printFloat(p, n); }
+
+  private:
+    size_t printNumber(Print*, unsigned long) const;
+    size_t printSignedNumber(Print*, signed long) const;
+    size_t printFloat(Print*, double) const;
+
+    uint8_t base;
+    uint8_t min_width;
+    uint8_t precision;
+
 };
-struct FormatOptionMinWidth : Print::FormatterOption<DefaultFormatter, uint8_t> {
-  constexpr FormatOptionMinWidth(uint8_t value) : Print::FormatterOption<DefaultFormatter, uint8_t>(value) { }
+
+/******************************************************************
+ * OptionList                                                     */
+
+template<typename THead, typename TTail = void>
+class OptionList;
+
+template<typename THead, typename TTail>
+class OptionList : public Print::FormatterOption {
+  public:
+    THead head;
+    TTail tail;
+
+    constexpr OptionList(const THead& head, const TTail& tail) : head(head), tail(tail) { }
+
+    // Apply our contained options to a formatter
+    template<typename TFormatter>
+    auto addToFormatter(const TFormatter& formatter) const
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)),
+                         decltype(formatter + this->head + this->tail)>
+    {
+      return formatter + this->head + this->tail;
+    }
+
+    // Make all OptionLists friends of us
+    template<typename T1, typename T2> friend class OptionList;
+
+  public:
+    // Append another option
+    template<typename TOption>
+    constexpr auto operator +(const TOption& option) const
+    -> enable_if_valid_t<decltype(accepts_option(&option)),
+                         OptionList<TOption, decltype(this->tail + option)>>
+    {
+      return {this->head, this->tail + option};
+    }
 };
 
-#undef HEX
-inline constexpr FormatOptionMinWidth PRINT_MIN_WIDTH(uint8_t min_width) { return {min_width}; }
-inline constexpr FormatOptionBase PRINT_BASE(uint8_t base) { return {base}; }
-constexpr FormatOptionBase HEX = PRINT_BASE(16);
-
-struct DefaultFormatter : Print::Formatter {
-  uint8_t base;
-  uint8_t min_width;
-
-  DefaultFormatter(uint8_t base = 10, uint8_t min_width = 0)
-    : base(base), min_width(min_width)
-  { }
-
-  DefaultFormatter addOption(FormatOptionBase o) const {
-    return {o.value, this->min_width};
-  }
-
-  DefaultFormatter addOption(FormatOptionMinWidth o) const {
-    return {this->base, o.value};
-  }
-
-  size_t printTo(Print *p, int n) const;
-  size_t printTo(Print *p, const char *) const;
-  template <typename T>
-  size_t printTo(Print *p, const T&) const;
+template<typename THead>
+struct OptionList<THead, void> : public Print::FormatterOption {
+  public:
+    THead head;
+
+    constexpr OptionList(const THead& head) : head(head) { }
+
+    // Apply our contained options to a formatter
+    template<typename TFormatter>
+    constexpr auto addToFormatter(const TFormatter& formatter) const
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)),
+                         decltype(formatter + this->head)>
+    {
+      return formatter + this->head;
+    }
+
+    // Make all OptionLists friends of us
+    template<typename T1, typename T2> friend class OptionList;
+
+  public:
+    // Append another option
+    template<typename TOption>
+    constexpr auto operator +(const TOption& option) const
+    -> enable_if_valid_t<decltype(accepts_option(&option)),
+                         OptionList<THead, OptionList<TOption, void>>>
+    {
+      return {this->head, option};
+    }
+
 };
 
-// TODO: These can probably be moved back inline above (they were split
-// when PrintHelper did not exist, so the Print class was defined
-// between the declaration of DefaultFormatter and these method
-// definitions).
-inline size_t DefaultFormatter::printTo(Print *p, int n) const {
-  return p->doPrint(n, this->base);
+// Apply an option list by adding it to a formatter (e.g. by passing it
+// to print())
+template<typename TFormatter, typename THead, typename TTail>
+constexpr auto operator +(const TFormatter& formatter, const OptionList<THead, TTail>& list)
+-> decltype(list.addToFormatter(formatter))
+{
+  return list.addToFormatter(formatter);
+}
+
+// Start an OptionList by adding two options
+template<typename TOption1, typename TOption2>
+constexpr auto operator+(const TOption1& first, const TOption2& second)
+-> enable_if_valid_t<decltype(accepts_option(&first), accepts_option(&second)),
+                     OptionList<TOption1, OptionList<TOption2, void>>>
+{
+  return {first, second};
+}
+
+// The DefaultFormatter for an option list is the DefaultFormatter for
+// the first option
+template<typename TValue, typename THead, typename TTail>
+auto DefaultFormatterFor(TValue value, OptionList<THead, TTail> list)
+-> decltype(DefaultFormatterFor(value, list.head)) {
+  return DefaultFormatterFor(value, list.head);
+}
+
+/* End of OptionList stuff                                        *
+ ******************************************************************/
+
+#if 0
+inline DefaultFormatter DefaultFormatterFor(Any /* value */, FormatOptionMinWidth) {
+  return {};
 }
 
-inline size_t DefaultFormatter::printTo(Print *p, const char * s) const {
-  // TODO: Maybe replace bool has_int_options with a more detailed bitwise flag int
-  // for better error messages.
-  //static_assert(!has_int_options, "Cannot print strings with integer-specific formatting options");
-  p->doPrint(s);
-  return 0;
+inline DefaultFormatter DefaultFormatterFor(Any /* value */, FormatOptionBase) {
+  return {};
+}
+#endif
+template<typename T>
+inline DefaultFormatter DefaultFormatterFor(T, DefaultFormatter::FormatterOption) {
+  return {};
 }
 
-template <typename T>
-inline size_t DefaultFormatter::printTo(Print *p, const T& v) const {
-  return p->doPrint(v);
+/*
+template<typename T>
+T declval();
+
+template<typename TValue,
+         typename TOption,
+         typename Test1 = decltype(declval<DefaultFormatter>().addOption(declval<TOption>())),
+         typename Test2 = decltype(declval<DefaultFormatter>().printTo(declval<Print*>(), declval<TValue>()))
+>
+inline DefaultFormatter DefaultFormatterFor(TValue, TOption) {
+  return {};
 }
+*/
+
+} // namespace Formatters
+
+#undef HEX
+inline constexpr Formatters::DefaultFormatter::FormatOptionMinWidth FORMAT_MIN_WIDTH(uint8_t min_width) { return {min_width}; }
+inline constexpr Formatters::DefaultFormatter::FormatOptionBase FORMAT_BASE(uint8_t base) { return {base}; }
+inline constexpr Formatters::DefaultFormatter::FormatOptionPrecision FORMAT_PRECISION(uint8_t prec) { return {prec}; }
+constexpr Formatters::DefaultFormatter::FormatOptionBase HEX = FORMAT_BASE(16);
+
+//constexpr auto FORMAT_PRECISION = Formatters::DefaultFormatter::FORMAT_PRECISION;
+
+inline size_t Print::print(  signed char  n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(  signed short n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(  signed int   n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(  signed long  n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(unsigned char  n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(unsigned short n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(unsigned int   n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(unsigned long  n, int base) { return print(n, FORMAT_BASE(base)); }
+inline size_t Print::print(    float      n, int prec) { return print(n, FORMAT_PRECISION(prec)); }
+inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_PRECISION(prec)); }
 
 // TODO: Should we really need PrintHelper? It was now added allow
 // referencing DefaultFormatter above. These methods could just be
@@ -280,49 +457,78 @@ class PrintHelper {
     static _always_inline size_t printTo(Print * p, const T &arg, const Ts &...args) {
       // This might lead to infinite template instantion
       //return print(arg, DefaultFormatter<>(), args...);
-      size_t n = DefaultFormatter().printTo(p, arg);
+      size_t n = Formatters::DefaultFormatter().printTo(p, arg);
       return n + printTo(p, args...);
     }
 /*
     template<typename T, typename T2, typename ...Ts>
     static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
-    -> check_type_t<decltype(arg2.printTo(p, arg)), size_t> {
+    -> enable_if_valid_t<decltype(arg2.printTo(p, arg)), size_t> {
       size_t n = arg2.printTo(p, arg);
       return n + printTo(p, args...);
     }
 */
-    static void accepts_formatter(const Print::Formatter*);
 
     template<typename T, typename T2, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
-    -> check_type_t<decltype(accepts_formatter(&arg2)), size_t> {
-    //-> check_type_t<decltype(arg2.printTo(p, arg)), size_t> {
-      size_t n = arg2.printTo(p, arg);
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &formatter, const Ts &...args)
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+    //-> enable_if_valid_t<decltype(formatter.printTo(p, arg)), size_t> {
+      size_t n = formatter.printTo(p, arg);
       return n + printTo(p, args...);
     }
 
 
-    template<typename T, typename T2, typename T3, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const T3 &arg3, const Ts &...args)
-    -> check_type_t<decltype(arg2.addOption(arg3)), size_t> {
-      return printTo(p, arg, arg2.addOption(arg3), args...);
+    template<typename T, typename TFormatter, typename TOption, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter.addOption(option)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter), accepts_option(&option)), size_t> {
+      //return printTo(p, arg, formatter.addOption(option), args...);
+      return printTo(p, arg, formatter + option, args...);
     }
 
-    static void accepts_option(const Print::FormatterOptionBase*);
 
     template<typename T, typename T2, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
-    //-> check_type_t<decltype(typename T2::Formatter().addOption(arg2)), size_t> {
-    -> check_type_t<decltype(accepts_option(&arg2)), size_t> {
-      //accepts_option(&arg2);
-      return printTo(p, arg, typename T2::Formatter().addOption(arg2), args...);
+    static _always_inline auto printTo(Print * p, const T &arg, const T2 &option, const Ts &...args)
+    //-> enable_if_valid_t<decltype(typename T2::Formatter().addOption(option)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_option(&option)), size_t> {
+      auto formatter = DefaultFormatterFor(arg, option);
+      //return printTo(p, arg, formatter + option, args...);
+      return printTo(p, arg, formatter, option, args...);
     }
 
+    // The next two overloads unpack an OptionList when it is
+    // supplied. These are not strictly needed (OptionList implements
+    // operator+ to apply each of its arguments to the formatter in
+    // turn), but without these the error messages are less clear if
+    // incompatible options are mixed (with the below overloads the
+    // error shows the formatter and the incompatible option, while
+    // without them only the formatter and the entire option list are
+    // shown.
+    //
+    // If we keep these, OptionList::addToFormatter and the related
+    // operator+ overload can be removed.
+    template<typename T, typename TFormatter, typename THead, typename TTail, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+      return printTo(p, arg, formatter, list.head, list.tail, args...);
+    }
+
+    // Base case for the end of the OptionList
+    template<typename T, typename TFormatter, typename THead, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, void> &list, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+      return printTo(p, arg, formatter, list.head, args...);
+    }
+
+    // Base case for the argument recursion - no more things to print
     static _always_inline size_t printTo(Print *) { return 0; }
 };
 
 template<typename ...Ts>
-_always_inline size_t Print::print(const Ts &...args) { return PrintHelper::printTo(this, args...); }
+inline _always_inline size_t Print::print(const Ts &...args) { return PrintHelper::printTo(this, args...); }
+
 #undef _always_inline
 
 #endif
@@ -341,3 +547,33 @@ _always_inline size_t Print::print(const Ts &...args) { return PrintHelper::prin
 // catch-all template), so that when you try printing any unsupported
 // types you still get the proper "no such method to call, candiates
 // are..." error message.
+//
+// Idea: Instead of using a Formatter and FormatterOption superclass,
+// create them as wrapper classes, so you can match them directly. Then
+// unpack and repack the values inside whenever you work with them.
+//
+// Idea: Expose accepts_option and accepts_formatter to debug problems
+// when formatters are not accepted (which typically leads to errors
+// that only say printTo(..., formatter/option) is not defined. A typical
+// error is no (or private) Formatter/FormatterOption base class.
+//
+// Mail
+//
+// For adding options to formatters, I'm using the overloaded addition
+// operator. This has the advantage that you can provide it inside the
+// formatter class, but also outside of it. This means you can
+// potentially define more formatter options, and apply them to an
+// existing formatter, without having to modify the formatter. If we
+// prefer a normal method/function, the same could probably be obtained
+// using a global overloaded addOption function (with a template version
+// that defers to an addOption method if it exists).
+//
+// Formatters and options are currently passed around as const
+// references, meaning that their printTo methods and addition operators
+// must be const methods. We can probably also provide non-const
+// reference and/or rvalue-reference versions of the variadic print
+// methods (probably even automatically template-generated by using &&
+// and std::forward), in case we want to allow implementing formatters
+// than modify themselves rather than return a new formatter (though I
+// can't really come up with a usecase for this - maybe a formatter that
+// counts characters written for alignment?).

From 288d94747219114562b9f1bf525e5f2ffdeae777 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Fri, 1 Jun 2018 18:24:36 +0200
Subject: [PATCH 09/19] WIP

---
 cores/arduino/Print.h | 73 +++++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 44 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 444327469..350840ce5 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -245,7 +245,7 @@ class DefaultFormatter : public Print::Formatter {
       uint8_t value;
     };
     constexpr DefaultFormatter operator+(FormatOptionPrecision o) {
-      return {this->base, o.value, this->precision};
+      return {this->base, this->min_width, o.value};
     }
 
     struct FormatOptionMinWidth : FormatterOption {
@@ -253,7 +253,7 @@ class DefaultFormatter : public Print::Formatter {
       uint8_t value;
     };
     constexpr DefaultFormatter operator+(FormatOptionMinWidth o) {
-      return {this->base, this->min_width, o.value};
+      return {this->base, o.value, this->precision};
     }
 
     constexpr DefaultFormatter(uint8_t base = 10, uint8_t min_width = 0, uint8_t precision = 2)
@@ -394,44 +394,26 @@ auto DefaultFormatterFor(TValue value, OptionList<THead, TTail> list)
 /* End of OptionList stuff                                        *
  ******************************************************************/
 
-#if 0
-inline DefaultFormatter DefaultFormatterFor(Any /* value */, FormatOptionMinWidth) {
-  return {};
-}
+} // namespace Formatters
 
-inline DefaultFormatter DefaultFormatterFor(Any /* value */, FormatOptionBase) {
-  return {};
-}
-#endif
 template<typename T>
-inline DefaultFormatter DefaultFormatterFor(T, DefaultFormatter::FormatterOption) {
+inline Formatters::DefaultFormatter DefaultFormatterFor(T, Formatters::DefaultFormatter::FormatterOption) {
   return {};
 }
 
-/*
 template<typename T>
-T declval();
-
-template<typename TValue,
-         typename TOption,
-         typename Test1 = decltype(declval<DefaultFormatter>().addOption(declval<TOption>())),
-         typename Test2 = decltype(declval<DefaultFormatter>().printTo(declval<Print*>(), declval<TValue>()))
->
-inline DefaultFormatter DefaultFormatterFor(TValue, TOption) {
+inline Formatters::DefaultFormatter DefaultFormatterFor(T) {
   return {};
 }
-*/
-
-} // namespace Formatters
 
 #undef HEX
 inline constexpr Formatters::DefaultFormatter::FormatOptionMinWidth FORMAT_MIN_WIDTH(uint8_t min_width) { return {min_width}; }
 inline constexpr Formatters::DefaultFormatter::FormatOptionBase FORMAT_BASE(uint8_t base) { return {base}; }
 inline constexpr Formatters::DefaultFormatter::FormatOptionPrecision FORMAT_PRECISION(uint8_t prec) { return {prec}; }
 constexpr Formatters::DefaultFormatter::FormatOptionBase HEX = FORMAT_BASE(16);
-
 //constexpr auto FORMAT_PRECISION = Formatters::DefaultFormatter::FORMAT_PRECISION;
 
+// Compatibility with previous versions, where base and precision were just numbers
 inline size_t Print::print(  signed char  n, int base) { return print(n, FORMAT_BASE(base)); }
 inline size_t Print::print(  signed short n, int base) { return print(n, FORMAT_BASE(base)); }
 inline size_t Print::print(  signed int   n, int base) { return print(n, FORMAT_BASE(base)); }
@@ -453,24 +435,24 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // declaration...).
 class PrintHelper {
   public:
+    // Base case for the argument recursion - no more things to print
+    static _always_inline size_t printTo(Print *) { return 0; }
+
+    // Simplest case: Just a value, no formatters or options (used when
+    // none of the below overloads match): Look up the default formatter
+    // for this value, and add it to the argument list.
     template<typename T, typename ...Ts>
     static _always_inline size_t printTo(Print * p, const T &arg, const Ts &...args) {
       // This might lead to infinite template instantion
       //return print(arg, DefaultFormatter<>(), args...);
-      size_t n = Formatters::DefaultFormatter().printTo(p, arg);
-      return n + printTo(p, args...);
-    }
-/*
-    template<typename T, typename T2, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &arg2, const Ts &...args)
-    -> enable_if_valid_t<decltype(arg2.printTo(p, arg)), size_t> {
-      size_t n = arg2.printTo(p, arg);
-      return n + printTo(p, args...);
+      return printTo(p, arg, DefaultFormatterFor(arg), args...);
     }
-*/
 
-    template<typename T, typename T2, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &formatter, const Ts &...args)
+    // A value and a formatter is specified without any options (or the
+    // below overloads have applied the options): Let the formatter
+    // print the value.
+    template<typename T, typename TFormatter, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Ts &...args)
     -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
     //-> enable_if_valid_t<decltype(formatter.printTo(p, arg)), size_t> {
       size_t n = formatter.printTo(p, arg);
@@ -478,21 +460,23 @@ class PrintHelper {
     }
 
 
+    // A value, a formatter and an option is specified: Apply the option
+    // to the formatter.
     template<typename T, typename TFormatter, typename TOption, typename ...Ts>
     static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter.addOption(option)), size_t> {
+    //-> enable_if_valid_t<decltype(formatter + option), size_t> {
     -> enable_if_valid_t<decltype(accepts_formatter(&formatter), accepts_option(&option)), size_t> {
-      //return printTo(p, arg, formatter.addOption(option), args...);
       return printTo(p, arg, formatter + option, args...);
     }
 
 
-    template<typename T, typename T2, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const T2 &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(typename T2::Formatter().addOption(option)), size_t> {
+    // A value and an option is specified: Look up the default formatter
+    // for this value and option and add it to the argument list.
+    template<typename T, typename TOption, typename ...Ts>
+    static _always_inline auto printTo(Print * p, const T &arg, const TOption &option, const Ts &...args)
+    //-> enable_if_valid_t<decltype(DefaultFormatterFor(arg, option)), size_t> {
     -> enable_if_valid_t<decltype(accepts_option(&option)), size_t> {
       auto formatter = DefaultFormatterFor(arg, option);
-      //return printTo(p, arg, formatter + option, args...);
       return printTo(p, arg, formatter, option, args...);
     }
 
@@ -507,6 +491,9 @@ class PrintHelper {
     //
     // If we keep these, OptionList::addToFormatter and the related
     // operator+ overload can be removed.
+    //
+    // If we add one more overload for (Value, OptionList, ...), we can
+    // also remove the DefaultFormatterForm definition for OptionList.
     template<typename T, typename TFormatter, typename THead, typename TTail, typename ...Ts>
     static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args)
     //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
@@ -522,8 +509,6 @@ class PrintHelper {
       return printTo(p, arg, formatter, list.head, args...);
     }
 
-    // Base case for the argument recursion - no more things to print
-    static _always_inline size_t printTo(Print *) { return 0; }
 };
 
 template<typename ...Ts>

From 9e132891ddf8e6dbaa64135f0c6729f831925818 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Fri, 1 Jun 2018 19:25:18 +0200
Subject: [PATCH 10/19] WIP - Remove PrintHelper

---
 cores/arduino/Print.h | 238 ++++++++++++++++++++----------------------
 1 file changed, 116 insertions(+), 122 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 350840ce5..a32c67f16 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -42,6 +42,17 @@ struct enable_if_valid {
 };
 template <typename Check, typename T> using enable_if_valid_t = typename enable_if_valid<Check, T>::type;
 
+/* Forward declarations */
+namespace Formatters {
+  template<typename THead, typename TTail = void>
+  class OptionList;
+
+  class DefaultFormatter;
+}
+
+template<typename T>
+Formatters::DefaultFormatter DefaultFormatterFor(T);
+
 class Print
 {
   private:
@@ -107,33 +118,11 @@ class Print
     }
     */
 
-#if __cplusplus >= 201103L
-    template<typename ...Ts> _always_inline size_t print(const Ts &...args);
+    template<typename ...Ts> _always_inline size_t print(const Ts &...args) { return printMultiple(args...); }
     template<typename ...Ts> _always_inline size_t println(const Ts &...args) { size_t t = print(args...); return t + println(); }
-#else
-    template<typename T> _always_inline size_t println(const T &arg)      { size_t t = print(arg);  return t + println(); }
-    template<typename T, typename T2> _always_inline size_t println(const T &arg1, const T2& arg2) { size_t t = print(arg1, arg2); return t + println(); }
-#endif // __cplusplus >= 201103L
 
     //_always_inline size_t print() { return 0; }
 
-    /** Variadic methods **/
-#if __cplusplus >= 201103L  // requires C++11
-/*
-    template<typename T, typename ...Ts>
-    _always_inline size_t print(const T &arg, const Ts &...args) {
-      size_t t = doPrint(arg);
-      return t + print(args...);
-    }
-
-    template<typename T, typename T2, typename ...Ts>
-    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> enable_if_valid_t<decltype(doPrint(arg, arg2)), size_t> {
-      size_t t = doPrint(arg, arg2);
-      return t + print(args...);
-    }
-*/
-
     // Compatibility versions of print that take a second base or
     // precision argument that is an integer.
     _always_inline size_t print(  signed char , int);
@@ -146,12 +135,84 @@ class Print
     _always_inline size_t print(unsigned long , int);
     _always_inline size_t print(    float     , int);
     _always_inline size_t print(    double    , int);
-#else
-    template<typename T> _always_inline size_t print(const T &arg)      { return doPrint(arg); }
-    template<typename T, typename T2> _always_inline size_t print(const T &arg1, const T2& arg2) { return doPrint(arg1, arg2); }
-#endif // __cplusplus >= 201103L
+
+  private:
+    // Base case for the argument recursion - no more things to print
+    _always_inline size_t printMultiple() { return 0; }
+
+    // Simplest case: Just a value, no formatters or options (used when
+    // none of the below overloads match): Look up the default formatter
+    // for this value, and add it to the argument list.
+    template<typename T, typename ...Ts>
+    _always_inline size_t printMultiple(const T &arg, const Ts &...args) {
+      // This might lead to infinite template instantion
+      //return print(arg, DefaultFormatter<>(), args...);
+      return printMultiple(arg, DefaultFormatterFor(arg), args...);
+    }
+
+    // A value and a formatter is specified without any options (or the
+    // below overloads have applied the options): Let the formatter
+    // print the value.
+    template<typename T, typename TFormatter, typename ...Ts>
+    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Ts &...args)
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+    //-> enable_if_valid_t<decltype(formatter.printTo(p, arg)), size_t> {
+      size_t n = formatter.printTo(this, arg);
+      return n + printMultiple(args...);
+    }
+
+
+    // A value, a formatter and an option is specified: Apply the option
+    // to the formatter.
+    template<typename T, typename TFormatter, typename TOption, typename ...Ts>
+    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter + option), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter), accepts_option(&option)), size_t> {
+      return printMultiple(arg, formatter + option, args...);
+    }
+
+
+    // A value and an option is specified: Look up the default formatter
+    // for this value and option and add it to the argument list.
+    template<typename T, typename TOption, typename ...Ts>
+    _always_inline auto printMultiple(const T &arg, const TOption &option, const Ts &...args)
+    //-> enable_if_valid_t<decltype(DefaultFormatterFor(arg, option)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_option(&option)), size_t> {
+      auto formatter = DefaultFormatterFor(arg, option);
+      return printMultiple(arg, formatter, option, args...);
+    }
+
+    // The next two overloads unpack an OptionList when it is
+    // supplied. These are not strictly needed (OptionList implements
+    // operator+ to apply each of its arguments to the formatter in
+    // turn), but without these the error messages are less clear if
+    // incompatible options are mixed (with the below overloads the
+    // error shows the formatter and the incompatible option, while
+    // without them only the formatter and the entire option list are
+    // shown.
+    //
+    // If we keep these, OptionList::addToFormatter and the related
+    // operator+ overload can be removed.
+    //
+    // If we add one more overload for (Value, OptionList, ...), we can
+    // also remove the DefaultFormatterForm definition for OptionList.
+    template<typename T, typename TFormatter, typename THead, typename TTail, typename ...Ts>
+    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+      return printMultiple(arg, formatter, list.head, list.tail, args...);
+    }
+
+    // Base case for the end of the OptionList
+    template<typename T, typename TFormatter, typename THead, typename ...Ts>
+    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, void> &list, const Ts &...args)
+    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
+    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+      return printMultiple(arg, formatter, list.head, args...);
+    }
 };
 
+
 void accepts_formatter(const Print::Formatter*);
 void accepts_option(const Print::FormatterOption*);
 
@@ -271,7 +332,7 @@ class DefaultFormatter : public Print::Formatter {
     _always_inline size_t printTo(Print* p, const char      c    ) const { return p->write(c); }
     _always_inline size_t printTo(Print* p, const Printable &x   ) const { return x.printTo(*p); }
 
-    /* Integer printing, upcast to (unsigned) long and then printed usign
+    /* Integer printing, upcast to (unsigned) long and then printed using
      * a shared print(Signed)Number function. */
     _always_inline size_t printTo(Print* p,   signed char  n) const { return printSignedNumber(p, n); }
     _always_inline size_t printTo(Print* p,   signed short n) const { return printSignedNumber(p, n); }
@@ -300,8 +361,8 @@ class DefaultFormatter : public Print::Formatter {
 /******************************************************************
  * OptionList                                                     */
 
-template<typename THead, typename TTail = void>
-class OptionList;
+//template<typename THead, typename TTail = void>
+//class OptionList;
 
 template<typename THead, typename TTail>
 class OptionList : public Print::FormatterOption {
@@ -413,7 +474,9 @@ inline constexpr Formatters::DefaultFormatter::FormatOptionPrecision FORMAT_PREC
 constexpr Formatters::DefaultFormatter::FormatOptionBase HEX = FORMAT_BASE(16);
 //constexpr auto FORMAT_PRECISION = Formatters::DefaultFormatter::FORMAT_PRECISION;
 
-// Compatibility with previous versions, where base and precision were just numbers
+// Compatibility versions of print that take a second base or
+// precision argument that is an integer. Defined here, since they need
+// to refer to DefaultFormatter options.
 inline size_t Print::print(  signed char  n, int base) { return print(n, FORMAT_BASE(base)); }
 inline size_t Print::print(  signed short n, int base) { return print(n, FORMAT_BASE(base)); }
 inline size_t Print::print(  signed int   n, int base) { return print(n, FORMAT_BASE(base)); }
@@ -425,95 +488,6 @@ inline size_t Print::print(unsigned long  n, int base) { return print(n, FORMAT_
 inline size_t Print::print(    float      n, int prec) { return print(n, FORMAT_PRECISION(prec)); }
 inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_PRECISION(prec)); }
 
-// TODO: Should we really need PrintHelper? It was now added allow
-// referencing DefaultFormatter above. These methods could just be
-// out-of-line definitions of methods in Print, but then the complicated
-// method signature must be duplicated. Alternatively, a single method
-// that generates a DefaultFormatter object could possibly be out of
-// line (though, thinking on it, both of these options might not work,
-// since they need DefaultFormatter as a return type in a
-// declaration...).
-class PrintHelper {
-  public:
-    // Base case for the argument recursion - no more things to print
-    static _always_inline size_t printTo(Print *) { return 0; }
-
-    // Simplest case: Just a value, no formatters or options (used when
-    // none of the below overloads match): Look up the default formatter
-    // for this value, and add it to the argument list.
-    template<typename T, typename ...Ts>
-    static _always_inline size_t printTo(Print * p, const T &arg, const Ts &...args) {
-      // This might lead to infinite template instantion
-      //return print(arg, DefaultFormatter<>(), args...);
-      return printTo(p, arg, DefaultFormatterFor(arg), args...);
-    }
-
-    // A value and a formatter is specified without any options (or the
-    // below overloads have applied the options): Let the formatter
-    // print the value.
-    template<typename T, typename TFormatter, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Ts &...args)
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
-    //-> enable_if_valid_t<decltype(formatter.printTo(p, arg)), size_t> {
-      size_t n = formatter.printTo(p, arg);
-      return n + printTo(p, args...);
-    }
-
-
-    // A value, a formatter and an option is specified: Apply the option
-    // to the formatter.
-    template<typename T, typename TFormatter, typename TOption, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter + option), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter), accepts_option(&option)), size_t> {
-      return printTo(p, arg, formatter + option, args...);
-    }
-
-
-    // A value and an option is specified: Look up the default formatter
-    // for this value and option and add it to the argument list.
-    template<typename T, typename TOption, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const TOption &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(DefaultFormatterFor(arg, option)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_option(&option)), size_t> {
-      auto formatter = DefaultFormatterFor(arg, option);
-      return printTo(p, arg, formatter, option, args...);
-    }
-
-    // The next two overloads unpack an OptionList when it is
-    // supplied. These are not strictly needed (OptionList implements
-    // operator+ to apply each of its arguments to the formatter in
-    // turn), but without these the error messages are less clear if
-    // incompatible options are mixed (with the below overloads the
-    // error shows the formatter and the incompatible option, while
-    // without them only the formatter and the entire option list are
-    // shown.
-    //
-    // If we keep these, OptionList::addToFormatter and the related
-    // operator+ overload can be removed.
-    //
-    // If we add one more overload for (Value, OptionList, ...), we can
-    // also remove the DefaultFormatterForm definition for OptionList.
-    template<typename T, typename TFormatter, typename THead, typename TTail, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
-      return printTo(p, arg, formatter, list.head, list.tail, args...);
-    }
-
-    // Base case for the end of the OptionList
-    template<typename T, typename TFormatter, typename THead, typename ...Ts>
-    static _always_inline auto printTo(Print * p, const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, void> &list, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
-      return printTo(p, arg, formatter, list.head, args...);
-    }
-
-};
-
-template<typename ...Ts>
-inline _always_inline size_t Print::print(const Ts &...args) { return PrintHelper::printTo(this, args...); }
-
 #undef _always_inline
 
 #endif
@@ -530,18 +504,36 @@ inline _always_inline size_t Print::print(const Ts &...args) { return PrintHelpe
 // template arguments. This can be solved by doing just the checks and
 // forwarding *all* printTo calls to another class (including a
 // catch-all template), so that when you try printing any unsupported
-// types you still get the proper "no such method to call, candiates
+// types you still get the proper "no such method to call, candidates
 // are..." error message.
 //
 // Idea: Instead of using a Formatter and FormatterOption superclass,
 // create them as wrapper classes, so you can match them directly. Then
-// unpack and repack the values inside whenever you work with them.
+// unpack and repack the values inside whenever you work with them. This
+// avoids the need for these somewhat clunky checks using
+// enable_if_valid_t and accepts_formatter. Note that we cannot just
+// declare the parameters to be of the superclass type, since we need to
+// know the actual formatter/option class too.
 //
 // Idea: Expose accepts_option and accepts_formatter to debug problems
 // when formatters are not accepted (which typically leads to errors
 // that only say printTo(..., formatter/option) is not defined. A typical
 // error is no (or private) Formatter/FormatterOption base class.
 //
+// Idea: Because DefaultFormatter::printTo accesses options through
+// this, the compiler seems to force the options onto the stack and
+// passes a pointer. It would probably be more efficient to just load
+// the options directly into registers instead. To do this,
+// all DefaultFormatter::printTo could be changed to static methods and
+// accept the DefaultFormatter instance by-value, and a generic
+// non-static printTo could be added that forwards all calls to the
+// static versions. Doing this in DefaultFormatter instead of coding
+// this in Print/PrintHelper, leaves control over by-value/by-ref at the
+// formatter author and probably gives the cleanest Formatter interface
+// (an alternative would to just have PrintHelper call a static printTo,
+// in which case the formatter could still decide to accept the
+// formatter instance by reference or by value).
+//
 // Mail
 //
 // For adding options to formatters, I'm using the overloaded addition
@@ -562,3 +554,5 @@ inline _always_inline size_t Print::print(const Ts &...args) { return PrintHelpe
 // than modify themselves rather than return a new formatter (though I
 // can't really come up with a usecase for this - maybe a formatter that
 // counts characters written for alignment?).
+//
+// Compatibility with C++ <= 11

From 3913a2b4cb79e8edbfbe39beb1185ab18252e65b Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 16:28:06 +0200
Subject: [PATCH 11/19] WIP

---
 cores/arduino/Print.h | 88 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 66 insertions(+), 22 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index a32c67f16..6544fd719 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -220,26 +220,6 @@ void accepts_option(const Print::FormatterOption*);
 // cluttering the global namespace. Could be removed if needed.
 namespace Formatters {
 
-  
-// TODO: Do we really need a FormatterOption base class? Without it,
-// options can be POD, not needing a constructor. With it, we can
-// prevent accidentally treating things as options which are not, but if
-// we already check a Formatter base class, we can also require that
-// Formatters do not define nonsensical addOption methods (even more,
-// without an explicit FormatterOption, Formatters can even use
-// non-class types as options if they want).
-// TODO: Where to define the "default formatter" for an option? Now, it
-// is our superclass, but it might just as well be defined with a using
-// directive directly here. Or perhaps it should be a method (this
-// might be problematic, since the DefaultFormatter type is incomplete
-// at this point). One completely different approach would be a
-// DefaultFormatterFor template, which gets specialized, but this
-// probably has the problem that once it is instantiated, you can no
-// longer add to it. Using specializations does allow defining a default
-// formatter for a type/option combination (allowing reuse of e.g. HEX
-// for custom types) or for just a type (allowing default formatting of
-// custom types).
-
 class DefaultFormatter : public Print::Formatter {
   public:
     // Common base class for our options
@@ -528,12 +508,61 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // accept the DefaultFormatter instance by-value, and a generic
 // non-static printTo could be added that forwards all calls to the
 // static versions. Doing this in DefaultFormatter instead of coding
-// this in Print/PrintHelper, leaves control over by-value/by-ref at the
+// this in Print, leaves control over by-value/by-ref at the
 // formatter author and probably gives the cleanest Formatter interface
-// (an alternative would to just have PrintHelper call a static printTo,
+// (an alternative would to just have Print call a static printTo,
 // in which case the formatter could still decide to accept the
 // formatter instance by reference or by value).
 //
+// Limitation: Default formatters for a value type and/or option type
+// are specified using overloads of the DefaultFormatterFor function.
+// You can use templates to get wildcard overloads (e.g. specify a
+// default formatter for all options passed to a specific type, or a
+// default formatter for any type combined with a specific formatter).
+//
+// If both overloads exist, this might cause ambiguity. e.g. when you
+// have a (FooT value, *) overload and a (*, BarOption) overload, doing
+// print(FooT(), BarOption()) is ambiguous. Usually, this will also be
+// invalid (the formatter belonging to BarOption will likely not know
+// how to print a FooT anyway). There might be cases where it is not,
+// but it is not clear which of the two to favor in this case. If
+// needed, some kind of priority tag argument could later be added, with
+// a fallback to the regular tag-less version). Also, even in the
+// invalid case, the "ambiguous" error message is not so nice.
+//
+// Note that the current approach of using a formatter-specific
+// superclass (e.g. DefaultFormatter::FormatterOption) between the
+// actual option class and Print::FormatterOption already makes
+// overloads that use it (and thus need parent class conversion) less
+// specific than templated overloads (which match their arguments
+// exactly). This resolves the ambiguity, but I'm not sure if this is
+// the right resolution.
+
+// Limitation: DefaultFormatterFor relies on argument-dependent-lookup
+// (ADL) to work properly. Normally, when a function is called, only
+// overloads defined up to where the function call is defined are
+// considered (e.g. the call the DefaultFormatterFor in Print.h). In
+// this case, we want to allow defining more overloads later. We can
+// make this work because DefaultFormatterFor is only called from
+// template functions, and for those ADL happens at template
+// *instantiation time*, rather than *definition time*.
+//
+// ADL happens for all types in a namespace (including the root
+// namespace). Notably, this means ADL happens for class types (options
+// and custom value types), but not for native types (e.g.  int does not
+// live in a namespace). In practice, this means that replacing the
+// default formatter for a native type (without any options) is not
+// possible, since the reference to e.g. DefaultFormatterFor(int) is
+// looked up at template definition time, not instantiation time.
+//
+// Two possible workarounds for this would be to add a wrapper class
+// around values before passing them to DefaultFormatterFor, or adding
+// an unused dummy argument that forces ADL. The latter is probably
+// easiest, and if the dummy argument is called NoOption and is the
+// second argument, that might actually be fairly easy to work with as
+// well.
+//
+//
 // Mail
 //
 // For adding options to formatters, I'm using the overloaded addition
@@ -556,3 +585,18 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // counts characters written for alignment?).
 //
 // Compatibility with C++ <= 11
+//
+// ADL needed for DefaultFormatterFor - no primitive types
+//
+// TODO: Suggest workaround for ADL/primitive types using wrapper type
+// (not in mail). Probably requires priority tagging to try wrapped type
+// before unwrapped type, or some kind of primitive/nonprimitive
+// detection for SFINAE. Or add Dummy argument, to force (meaningless)
+// ADL.
+//
+// TODO: See if accepts_formatter can be called in default template
+// argument as well (these are filled in after deduction, right? We only
+// depend on the type, not the argument value)?
+//
+// TODO: Use NoOption dummy argument to DefaultFormatterFor to force
+// ADL?

From 51bc0943c7fb8737e9c319c919f4c62be475cbd7 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 18:13:14 +0200
Subject: [PATCH 12/19] WIP: Convert to use enable_if_base_of

---
 cores/arduino/Print.h | 171 +++++++++++++++++++++++-------------------
 1 file changed, 93 insertions(+), 78 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 6544fd719..6aa316f04 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -42,6 +42,27 @@ struct enable_if_valid {
 };
 template <typename Check, typename T> using enable_if_valid_t = typename enable_if_valid<Check, T>::type;
 
+
+namespace detail {
+  // Returns a value of the given type. No implementation, so this is
+  // meant for use in decltype only. Identical to std::declval.
+  template <typename T> T declval();
+
+  // Function that accept a pointer of the given type. Can be used to
+  // detect if another type is convertible to T.
+  template<typename T>
+  void accepts_pointer_of_type(const T*);
+
+  // Simplified and integrated version of std::enable_if_t and
+  // std::is_base_of (which are not available on AVR). Integrating them
+  // makes things more specific and thus simpler.
+  // TODO: This check does not detect private base classes. To do so, a
+  // more complicated check is needed, something like
+  // https://stackoverflow.com/questions/2910979/how-does-is-base-of-work
+  template<typename Base, typename Derived>
+  using enable_if_base_of = decltype(accepts_pointer_of_type<Base>(declval<Derived*>()));
+}
+
 /* Forward declarations */
 namespace Formatters {
   template<typename THead, typename TTail = void>
@@ -89,34 +110,6 @@ class Print
     size_t println(void);
 
     virtual void flush() { /* Empty implementation for backward compatibility */ }
-    /*
-    template<typename T, typename ...Ts>
-    _always_inline size_t print(const T &arg, const Ts &...args) {
-      // This might lead to infinite template instantion
-      //return print(arg, DefaultFormatter<>(), args...);
-      size_t n = DefaultFormatter<>().printTo(this, arg);
-      return n + print(args...);
-    }
-
-    template<typename T, typename T2, typename ...Ts>
-    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> enable_if_valid_t<decltype(arg2.printTo(this, arg)), size_t> {
-      size_t n = arg2.printTo(this, arg);
-      return n + print(args...);
-    }
-
-    template<typename T, typename T2, typename T3, typename ...Ts>
-    _always_inline auto print(const T &arg, const T2 &arg2, const T3 &arg3, const Ts &...args)
-    -> enable_if_valid_t<decltype(arg2.addOption(arg3)), size_t> {
-      return print(arg, arg2.addOption(arg3), args...);
-    }
-
-    template<typename T, typename T2, typename ...Ts>
-    _always_inline auto print(const T &arg, const T2 &arg2, const Ts &...args)
-    -> enable_if_valid_t<decltype(DefaultFormatter<>().addOption(arg2)), size_t> {
-      return print(arg, DefaultFormatter<>().addOption(arg2), args...);
-    }
-    */
 
     template<typename ...Ts> _always_inline size_t print(const Ts &...args) { return printMultiple(args...); }
     template<typename ...Ts> _always_inline size_t println(const Ts &...args) { size_t t = print(args...); return t + println(); }
@@ -153,31 +146,40 @@ class Print
     // A value and a formatter is specified without any options (or the
     // below overloads have applied the options): Let the formatter
     // print the value.
-    template<typename T, typename TFormatter, typename ...Ts>
-    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Ts &...args)
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
-    //-> enable_if_valid_t<decltype(formatter.printTo(p, arg)), size_t> {
+    template<
+      typename T,
+      typename TFormatter,
+      typename ...Ts,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr
+    >
+    _always_inline size_t printMultiple(const T &arg, const TFormatter &formatter, const Ts &...args) {
       size_t n = formatter.printTo(this, arg);
       return n + printMultiple(args...);
     }
 
-
     // A value, a formatter and an option is specified: Apply the option
     // to the formatter.
-    template<typename T, typename TFormatter, typename TOption, typename ...Ts>
-    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter + option), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter), accepts_option(&option)), size_t> {
+    template<
+      typename T,
+      typename TFormatter,
+      typename TOption,
+      typename ...Ts,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr,
+      detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
+    >
+    _always_inline size_t printMultiple(const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args) {
       return printMultiple(arg, formatter + option, args...);
     }
 
-
     // A value and an option is specified: Look up the default formatter
     // for this value and option and add it to the argument list.
-    template<typename T, typename TOption, typename ...Ts>
-    _always_inline auto printMultiple(const T &arg, const TOption &option, const Ts &...args)
-    //-> enable_if_valid_t<decltype(DefaultFormatterFor(arg, option)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_option(&option)), size_t> {
+    template<
+      typename T,
+      typename TOption,
+      typename ...Ts,
+      detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
+    >
+    _always_inline size_t printMultiple(const T &arg, const TOption &option, const Ts &...args) {
       auto formatter = DefaultFormatterFor(arg, option);
       return printMultiple(arg, formatter, option, args...);
     }
@@ -187,8 +189,7 @@ class Print
     // operator+ to apply each of its arguments to the formatter in
     // turn), but without these the error messages are less clear if
     // incompatible options are mixed (with the below overloads the
-    // error shows the formatter and the incompatible option, while
-    // without them only the formatter and the entire option list are
+    // error shows the formatter and the incompatible option, while // without them only the formatter and the entire option list are
     // shown.
     //
     // If we keep these, OptionList::addToFormatter and the related
@@ -196,18 +197,27 @@ class Print
     //
     // If we add one more overload for (Value, OptionList, ...), we can
     // also remove the DefaultFormatterForm definition for OptionList.
-    template<typename T, typename TFormatter, typename THead, typename TTail, typename ...Ts>
-    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+    template<
+      typename T,
+      typename TFormatter,
+      typename THead,
+      typename TTail,
+      typename ...Ts,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr
+    >
+    _always_inline size_t printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, TTail> &list, const Ts &...args) {
       return printMultiple(arg, formatter, list.head, list.tail, args...);
     }
 
     // Base case for the end of the OptionList
-    template<typename T, typename TFormatter, typename THead, typename ...Ts>
-    _always_inline auto printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, void> &list, const Ts &...args)
-    //-> enable_if_valid_t<decltype(formatter.addOption(list.head)), size_t> {
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)), size_t> {
+    template<
+      typename T,
+      typename TFormatter,
+      typename THead,
+      typename ...Ts,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr
+    >
+    _always_inline size_t printMultiple(const T &arg, const TFormatter &formatter, const Formatters::OptionList<THead, void> &list, const Ts &...args) {
       return printMultiple(arg, formatter, list.head, args...);
     }
 };
@@ -341,9 +351,6 @@ class DefaultFormatter : public Print::Formatter {
 /******************************************************************
  * OptionList                                                     */
 
-//template<typename THead, typename TTail = void>
-//class OptionList;
-
 template<typename THead, typename TTail>
 class OptionList : public Print::FormatterOption {
   public:
@@ -353,10 +360,12 @@ class OptionList : public Print::FormatterOption {
     constexpr OptionList(const THead& head, const TTail& tail) : head(head), tail(tail) { }
 
     // Apply our contained options to a formatter
-    template<typename TFormatter>
+    template<
+      typename TFormatter,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr
+    >
     auto addToFormatter(const TFormatter& formatter) const
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)),
-                         decltype(formatter + this->head + this->tail)>
+    -> decltype(formatter + this->head + this->tail)
     {
       return formatter + this->head + this->tail;
     }
@@ -366,10 +375,12 @@ class OptionList : public Print::FormatterOption {
 
   public:
     // Append another option
-    template<typename TOption>
+    template<
+      typename TOption,
+      detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
+    >
     constexpr auto operator +(const TOption& option) const
-    -> enable_if_valid_t<decltype(accepts_option(&option)),
-                         OptionList<TOption, decltype(this->tail + option)>>
+    -> OptionList<TOption, decltype(this->tail + option)>
     {
       return {this->head, this->tail + option};
     }
@@ -383,10 +394,12 @@ struct OptionList<THead, void> : public Print::FormatterOption {
     constexpr OptionList(const THead& head) : head(head) { }
 
     // Apply our contained options to a formatter
-    template<typename TFormatter>
+    template<
+      typename TFormatter,
+      detail::enable_if_base_of<Print::Formatter, TFormatter>* = nullptr
+    >
     constexpr auto addToFormatter(const TFormatter& formatter) const
-    -> enable_if_valid_t<decltype(accepts_formatter(&formatter)),
-                         decltype(formatter + this->head)>
+    -> decltype(formatter + this->head)
     {
       return formatter + this->head;
     }
@@ -396,10 +409,12 @@ struct OptionList<THead, void> : public Print::FormatterOption {
 
   public:
     // Append another option
-    template<typename TOption>
+    template<
+      typename TOption,
+      detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
+    >
     constexpr auto operator +(const TOption& option) const
-    -> enable_if_valid_t<decltype(accepts_option(&option)),
-                         OptionList<THead, OptionList<TOption, void>>>
+    -> OptionList<THead, OptionList<TOption, void>>
     {
       return {this->head, option};
     }
@@ -416,10 +431,14 @@ constexpr auto operator +(const TFormatter& formatter, const OptionList<THead, T
 }
 
 // Start an OptionList by adding two options
-template<typename TOption1, typename TOption2>
+template<
+  typename TOption1,
+  typename TOption2,
+  detail::enable_if_base_of<Print::FormatterOption, TOption1>* = nullptr
+  detail::enable_if_base_of<Print::FormatterOption, TOption2>* = nullptr
+>
 constexpr auto operator+(const TOption1& first, const TOption2& second)
--> enable_if_valid_t<decltype(accepts_option(&first), accepts_option(&second)),
-                     OptionList<TOption1, OptionList<TOption2, void>>>
+-> OptionList<TOption1, OptionList<TOption2, void>>
 {
   return {first, second};
 }
@@ -491,14 +510,10 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // create them as wrapper classes, so you can match them directly. Then
 // unpack and repack the values inside whenever you work with them. This
 // avoids the need for these somewhat clunky checks using
-// enable_if_valid_t and accepts_formatter. Note that we cannot just
-// declare the parameters to be of the superclass type, since we need to
-// know the actual formatter/option class too.
-//
-// Idea: Expose accepts_option and accepts_formatter to debug problems
-// when formatters are not accepted (which typically leads to errors
-// that only say printTo(..., formatter/option) is not defined. A typical
-// error is no (or private) Formatter/FormatterOption base class.
+// enable_if_base_of. Declaring an option would then be a bit more
+// involved and less intuitive, though. Note that we cannot just declare
+// the parameters to be of the superclass type, since we need to know
+// the actual formatter/option class too.
 //
 // Idea: Because DefaultFormatter::printTo accesses options through
 // this, the compiler seems to force the options onto the stack and

From 5ec021abfa62f9b74ab97119fc99b11e822d449b Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 18:14:01 +0200
Subject: [PATCH 13/19] WIP: Remove enable_if_valid_t

---
 cores/arduino/Print.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 6aa316f04..52c0beb3d 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -36,13 +36,6 @@
 
 #define _always_inline __attribute__ ((__always_inline__)) // undefined at end
 
-template <typename Check, typename T>
-struct enable_if_valid {
-  using type = T;
-};
-template <typename Check, typename T> using enable_if_valid_t = typename enable_if_valid<Check, T>::type;
-
-
 namespace detail {
   // Returns a value of the given type. No implementation, so this is
   // meant for use in decltype only. Identical to std::declval.

From 0a4b5029fd35e6401371fd262b4e04ddc1bb36df Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 18:28:34 +0200
Subject: [PATCH 14/19] fix

---
 cores/arduino/Print.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 52c0beb3d..aa8fb5771 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -427,7 +427,7 @@ constexpr auto operator +(const TFormatter& formatter, const OptionList<THead, T
 template<
   typename TOption1,
   typename TOption2,
-  detail::enable_if_base_of<Print::FormatterOption, TOption1>* = nullptr
+  detail::enable_if_base_of<Print::FormatterOption, TOption1>* = nullptr,
   detail::enable_if_base_of<Print::FormatterOption, TOption2>* = nullptr
 >
 constexpr auto operator+(const TOption1& first, const TOption2& second)

From 18b59972194aa5a6787cb76d5ce2688b81e82413 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 18:28:38 +0200
Subject: [PATCH 15/19] Convert operator+ to applyOption function/method

---
 cores/arduino/Print.h | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index aa8fb5771..abb089743 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -161,7 +161,7 @@ class Print
       detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
     >
     _always_inline size_t printMultiple(const T &arg, const TFormatter &formatter, const TOption &option, const Ts &...args) {
-      return printMultiple(arg, formatter + option, args...);
+      return printMultiple(arg, applyOption(formatter, option), args...);
     }
 
     // A value and an option is specified: Look up the default formatter
@@ -219,6 +219,16 @@ class Print
 void accepts_formatter(const Print::Formatter*);
 void accepts_option(const Print::FormatterOption*);
 
+// Global function to apply an option on a given formatter. This version
+// just calls the applyOption method on the formatter, if it exists. By
+// going through this global function, it is possible to add overloads
+// to add options for an existing formatter.
+template <typename TFormatter, typename TOption>
+_always_inline inline auto applyOption(const TFormatter& formatter, const TOption& option)
+-> decltype(formatter.applyOption(option)) {
+  return formatter.applyOption(option);
+}
+
 // Namespace for more advanced custom formatter stuff, to prevent
 // cluttering the global namespace. Could be removed if needed.
 namespace Formatters {
@@ -280,7 +290,7 @@ class DefaultFormatter : public Print::Formatter {
       constexpr FormatOptionBase(uint8_t value) : value(value) { }
       uint8_t value;
     };
-    constexpr DefaultFormatter operator+(FormatOptionBase o) {
+    constexpr DefaultFormatter applyOption(FormatOptionBase o) {
       return {o.value, this->min_width, this->precision};
     }
 
@@ -288,7 +298,7 @@ class DefaultFormatter : public Print::Formatter {
       constexpr FormatOptionPrecision(uint8_t value) : value(value) { }
       uint8_t value;
     };
-    constexpr DefaultFormatter operator+(FormatOptionPrecision o) {
+    constexpr DefaultFormatter applyOption(FormatOptionPrecision o) {
       return {this->base, this->min_width, o.value};
     }
 
@@ -296,7 +306,7 @@ class DefaultFormatter : public Print::Formatter {
       constexpr FormatOptionMinWidth(uint8_t value) : value(value) { }
       uint8_t value;
     };
-    constexpr DefaultFormatter operator+(FormatOptionMinWidth o) {
+    constexpr DefaultFormatter applyOption(FormatOptionMinWidth o) {
       return {this->base, o.value, this->precision};
     }
 
@@ -372,7 +382,7 @@ class OptionList : public Print::FormatterOption {
       typename TOption,
       detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
     >
-    constexpr auto operator +(const TOption& option) const
+    constexpr auto operator+(const TOption& option) const
     -> OptionList<TOption, decltype(this->tail + option)>
     {
       return {this->head, this->tail + option};
@@ -406,7 +416,7 @@ struct OptionList<THead, void> : public Print::FormatterOption {
       typename TOption,
       detail::enable_if_base_of<Print::FormatterOption, TOption>* = nullptr
     >
-    constexpr auto operator +(const TOption& option) const
+    constexpr auto operator+(const TOption& option) const
     -> OptionList<THead, OptionList<TOption, void>>
     {
       return {this->head, option};
@@ -417,7 +427,7 @@ struct OptionList<THead, void> : public Print::FormatterOption {
 // Apply an option list by adding it to a formatter (e.g. by passing it
 // to print())
 template<typename TFormatter, typename THead, typename TTail>
-constexpr auto operator +(const TFormatter& formatter, const OptionList<THead, TTail>& list)
+constexpr auto applyOption(const TFormatter& formatter, const OptionList<THead, TTail>& list)
 -> decltype(list.addToFormatter(formatter))
 {
   return list.addToFormatter(formatter);
@@ -580,7 +590,12 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // existing formatter, without having to modify the formatter. If we
 // prefer a normal method/function, the same could probably be obtained
 // using a global overloaded addOption function (with a template version
-// that defers to an addOption method if it exists).
+// that defers to an addOption method if it exists). After writing this,
+// I realized that a method/function name is probably gives more
+// readable errors (e.g. "no such function applyOption(DefaultFormatter,
+// SomeOption)", rather than "no such operator
+// operator+(DefaultFormatter, SomeOption)"), so we should probably do
+// that.
 //
 // Formatters and options are currently passed around as const
 // references, meaning that their printTo methods and addition operators
@@ -596,15 +611,9 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 //
 // ADL needed for DefaultFormatterFor - no primitive types
 //
-// TODO: Suggest workaround for ADL/primitive types using wrapper type
-// (not in mail). Probably requires priority tagging to try wrapped type
-// before unwrapped type, or some kind of primitive/nonprimitive
-// detection for SFINAE. Or add Dummy argument, to force (meaningless)
-// ADL.
-//
-// TODO: See if accepts_formatter can be called in default template
-// argument as well (these are filled in after deduction, right? We only
-// depend on the type, not the argument value)?
+// Compare with Printable
 //
 // TODO: Use NoOption dummy argument to DefaultFormatterFor to force
 // ADL?
+//
+// TODO: Convert operator+ to applyOption?

From 07267a11a0b37488156b92f4deb2a3a1867a3816 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 4 Jun 2018 19:03:57 +0200
Subject: [PATCH 16/19] Very rough FORMAT_MIN_WIDTH implementation

---
 cores/arduino/Print.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cores/arduino/Print.cpp b/cores/arduino/Print.cpp
index 00c1b12b7..86cbc98b1 100644
--- a/cores/arduino/Print.cpp
+++ b/cores/arduino/Print.cpp
@@ -99,6 +99,9 @@ size_t Formatters::DefaultFormatter::printNumber(Print *p , unsigned long n) con
     *--str = c < 10 ? c + '0' : c + 'A' - 10;
   } while(n);
 
+  while (str > buf && buf + sizeof(buf) - 1 - str < this->min_width)
+    *--str = '0';
+
   return p->write(str);
 }
 

From 11a3dc953863f1346b90456acdfc5090ca1e65a8 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Tue, 12 Jun 2018 11:26:34 +0200
Subject: [PATCH 17/19] WIP comments

---
 cores/arduino/Print.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index abb089743..933f9888a 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -613,7 +613,19 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 //
 // Compare with Printable
 //
+// Customization: Custom formatter (with or without options),
+// optionlist, custom option for existing formatter using applyOption
+// overload.
+//
+// Error messages (a bit fuzzy due to "when instantiating" clauses, but
+// the actual error is usually ok. A lot of gunk follows, though).
+//
 // TODO: Use NoOption dummy argument to DefaultFormatterFor to force
 // ADL?
 //
 // TODO: Convert operator+ to applyOption?
+//
+// TODO: Naming of option classes (shows up in error message)
+//
+// TODO: Getters and setters for DefaultFormatter options? Helps with
+// extending.

From 67ad205437ed0299c0c0dbc3207f0f8f90e41bdf Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Mon, 24 Sep 2018 16:30:35 +0200
Subject: [PATCH 18/19] WIP comments

---
 cores/arduino/Print.h | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 933f9888a..885078e8b 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -36,19 +36,25 @@
 
 #define _always_inline __attribute__ ((__always_inline__)) // undefined at end
 
+
+// Namespace to hide implementation details into. Its contents should
+// not be used outside of this file.
 namespace detail {
   // Returns a value of the given type. No implementation, so this is
   // meant for use in decltype only. Identical to std::declval.
   template <typename T> T declval();
 
-  // Function that accept a pointer of the given type. Can be used to
-  // detect if another type is convertible to T.
+  // Function that accepts a pointer of the given type. Can be used to
+  // detect if another type is convertible to T. No implementation, so
+  // intended to be used in declval only.
   template<typename T>
   void accepts_pointer_of_type(const T*);
 
   // Simplified and integrated version of std::enable_if_t and
   // std::is_base_of (which are not available on AVR). Integrating them
   // makes things more specific and thus simpler.
+  // This type alias resolves to `void` when `Base` is a base class of
+  // `Derived`, or fails to resolve otherwise.
   // TODO: This check does not detect private base classes. To do so, a
   // more complicated check is needed, something like
   // https://stackoverflow.com/questions/2910979/how-does-is-base-of-work
@@ -64,6 +70,7 @@ namespace Formatters {
   class DefaultFormatter;
 }
 
+// Forward declaration
 template<typename T>
 Formatters::DefaultFormatter DefaultFormatterFor(T);
 
@@ -182,14 +189,15 @@ class Print
     // operator+ to apply each of its arguments to the formatter in
     // turn), but without these the error messages are less clear if
     // incompatible options are mixed (with the below overloads the
-    // error shows the formatter and the incompatible option, while // without them only the formatter and the entire option list are
+    // error shows the formatter and the incompatible option, while
+    // without them only the formatter and the entire option list are
     // shown.
     //
     // If we keep these, OptionList::addToFormatter and the related
     // operator+ overload can be removed.
     //
     // If we add one more overload for (Value, OptionList, ...), we can
-    // also remove the DefaultFormatterForm definition for OptionList.
+    // also remove the DefaultFormatterFor definition for OptionList.
     template<
       typename T,
       typename TFormatter,
@@ -459,17 +467,31 @@ auto DefaultFormatterFor(TValue value, OptionList<THead, TTail> list)
 
 } // namespace Formatters
 
+// The DefaultFormatterFor function is used to define what formatter to
+// use for a value when no formatter is explicitly specified. This is
+// primarily intended to pick a formatter based on the value type (first
+// argument), but it is also possible to look at the value itself when
+// building the formatter. Note that it is not possible to return a
+// different *type* of formatter based on the value, it is possible to
+// set options in the formatter based on the value.
+// TODO: Document ADL stuff?
 template<typename T>
-inline Formatters::DefaultFormatter DefaultFormatterFor(T, Formatters::DefaultFormatter::FormatterOption) {
+inline Formatters::DefaultFormatter DefaultFormatterFor(T) {
   return {};
 }
 
+// Overload that decides on a formatter to use based on the value to
+// print as well as the (first) formatter option passed. See
+// `DefaultFormatterFor(T)` overload for more details.
 template<typename T>
-inline Formatters::DefaultFormatter DefaultFormatterFor(T) {
+inline Formatters::DefaultFormatter DefaultFormatterFor(T, Formatters::DefaultFormatter::FormatterOption) {
   return {};
 }
 
+// TODO: Fix this in Arduino.h
 #undef HEX
+
+// TODO: Finalize options
 inline constexpr Formatters::DefaultFormatter::FormatOptionMinWidth FORMAT_MIN_WIDTH(uint8_t min_width) { return {min_width}; }
 inline constexpr Formatters::DefaultFormatter::FormatOptionBase FORMAT_BASE(uint8_t base) { return {base}; }
 inline constexpr Formatters::DefaultFormatter::FormatOptionPrecision FORMAT_PRECISION(uint8_t prec) { return {prec}; }
@@ -571,7 +593,8 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // live in a namespace). In practice, this means that replacing the
 // default formatter for a native type (without any options) is not
 // possible, since the reference to e.g. DefaultFormatterFor(int) is
-// looked up at template definition time, not instantiation time.
+// looked up in the set of overloads that were defined at template
+// definition time, not at instantiation time.
 //
 // Two possible workarounds for this would be to add a wrapper class
 // around values before passing them to DefaultFormatterFor, or adding
@@ -596,6 +619,7 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // SomeOption)", rather than "no such operator
 // operator+(DefaultFormatter, SomeOption)"), so we should probably do
 // that.
+// TODO: This was changed to applyOption, reword
 //
 // Formatters and options are currently passed around as const
 // references, meaning that their printTo methods and addition operators
@@ -623,8 +647,6 @@ inline size_t Print::print(    double     n, int prec) { return print(n, FORMAT_
 // TODO: Use NoOption dummy argument to DefaultFormatterFor to force
 // ADL?
 //
-// TODO: Convert operator+ to applyOption?
-//
 // TODO: Naming of option classes (shows up in error message)
 //
 // TODO: Getters and setters for DefaultFormatter options? Helps with

From f77072648d055108c8663b0aaf4aaca8bf8a13e0 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Tue, 2 Oct 2018 09:26:47 +0200
Subject: [PATCH 19/19] WIP: Comment

---
 cores/arduino/Print.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/cores/arduino/Print.h b/cores/arduino/Print.h
index 885078e8b..d9c71df90 100644
--- a/cores/arduino/Print.h
+++ b/cores/arduino/Print.h
@@ -475,6 +475,12 @@ auto DefaultFormatterFor(TValue value, OptionList<THead, TTail> list)
 // different *type* of formatter based on the value, it is possible to
 // set options in the formatter based on the value.
 // TODO: Document ADL stuff?
+// TODO: Should this use a reference argument? Using a value argument
+// requires a copy constructor (even when a value *can* be printed
+// through an implicit conversion to another printable type). Can we
+// somehow apply this implicit conversion for DefaultFormatterFor too?
+// Otherwise you'd get the formatter for the original type, whereas
+// there might be a different formatter for the converted type?
 template<typename T>
 inline Formatters::DefaultFormatter DefaultFormatterFor(T) {
   return {};