Fix printing of collections. Make it impossible for a collection with 1 element to match the string of an empty collection. Avoid checking if comma needs to be added in loop. Make an empty tuple print same as empty collection: '{}'.

Signed-off-by: Vassil Vassilev <vvasilev@cern.ch>

include/cling/Interpreter/RuntimePrintValue.h

@@ -78,59 +78,18 @@ namespace cling {
   // cling::Value
   std::string printValue(const Value *value);
 
-  // Collections internal declaration
-  namespace collectionPrinterInternal {
-    // Maps declaration
-    template<typename CollectionType>
-    auto printValue_impl(const CollectionType *obj, short)
-      -> decltype(
-      ++(obj->begin()), obj->end(),
-        obj->begin()->first, obj->begin()->second,
-        std::string());
-
-    // Vector, set, deque etc. declaration.
-    template<typename CollectionType>
-    auto printValue_impl(const CollectionType *obj, int)
-      -> decltype(
-                  ++(obj->begin()), obj->end(),
-                  *(obj->begin()),  &(*(obj->begin())),
-                  std::string());
-
-    // As above, but without ability to take address of elements.
-    template<typename CollectionType>
-    auto printValue_impl(const CollectionType *obj, long)
-      -> decltype(
-                  ++(obj->begin()), obj->end(),
-                  *(obj->begin()),
-                  std::string());
-
-    // No general fallback anymore here, void* overload used for that now
-  }
-
-  // Collections
-  template<typename CollectionType>
-  auto printValue(const CollectionType *obj)
-  -> decltype(collectionPrinterInternal::printValue_impl(obj, 0), std::string())
-  {
-    return collectionPrinterInternal::printValue_impl(obj, (short)0);  // short -> int -> long = priority order
-  }
-
-  // Arrays
-  template<typename T, size_t N>
-  std::string printValue(const T (*obj)[N]) {
-    std::string str = "{ ";
-
-    for (int i = 0; i < N; ++i) {
-      str += printValue(*obj + i);
-      if (i < N - 1) str += ", ";
-    }
-
-    return str + " }";
+  namespace valuePrinterInternal {
+    extern const char* const kEmptyCollection;
   }
 
   // Collections internal
   namespace collectionPrinterInternal {
     // Maps
+    template<typename Pair>
+    std::string printPair(const Pair& P) {
+      return printValue(&P->first) + " => " + printValue(&P->second);
+    }
+    
     template<typename CollectionType>
     auto printValue_impl(const CollectionType *obj, short)
     -> decltype(
@@ -138,20 +97,17 @@ namespace cling {
         obj->begin()->first, obj->begin()->second,
         std::string())
     {
-      std::string str = "{ ";
-
       auto iter = obj->begin();
       auto iterEnd = obj->end();
-      while (iter != iterEnd) {
-        str += printValue(&iter->first);
-        str += " => ";
-        str += printValue(&iter->second);
-        ++iter;
-        if (iter != iterEnd) {
-          str += ", ";
-        }
-      }
+      if (iter == iterEnd)
+        return valuePrinterInternal::kEmptyCollection;
 
+      std::string str("{ ");
+      str += printPair(*iter);
+      while (++iter != iterEnd) {
+        str += ", ";
+        str += printPair(*iter);
+      }
       return str + " }";
     }
 
@@ -163,18 +119,17 @@ namespace cling {
         *(obj->begin()),  &(*(obj->begin())),
         std::string())
     {
-      std::string str = "{ ";
-
       auto iter = obj->begin();
       auto iterEnd = obj->end();
-      while (iter != iterEnd) {
-        str += printValue(&(*iter));
-        ++iter;
-        if (iter != iterEnd) {
-          str += ", ";
-        }
-      }
+      if (iter == iterEnd)
+        return valuePrinterInternal::kEmptyCollection;
 
+      std::string str("{ ");
+      str += printValue(&(*iter));
+      while (++iter != iterEnd) {
+        str += ", ";
+        str += printValue(&(*iter));
+      }
       return str + " }";
     }
 
@@ -186,22 +141,45 @@ namespace cling {
         *(obj->begin()),
         std::string())
      {
-        std::string str = "{ ";
+      auto iter = obj->begin();
+      auto iterEnd = obj->end();
+      if (iter == iterEnd)
+        return valuePrinterInternal::kEmptyCollection;
 
-        auto iter = obj->begin();
-        auto iterEnd = obj->end();
-        while (iter != iterEnd) {
-           const auto value = (*iter);
-           str += printValue(&value);
-           ++iter;
-           if (iter != iterEnd) {
-              str += ", ";
-           }
-        }
-
-        return str + " }";
+      std::string str("{ ");
+      const auto value0 = (*iter);
+      str += printValue(&value0);
+      while (++iter != iterEnd) {
+        const auto valueN = (*iter);
+        str += ", ";
+        str += printValue(&valueN);
+      }
+      return str + " }";
      }
+  }
 
+  // Collections
+  template<typename CollectionType>
+  auto printValue(const CollectionType *obj)
+  -> decltype(collectionPrinterInternal::printValue_impl(obj, 0), std::string())
+  {
+    // short -> int -> long = priority order
+    return collectionPrinterInternal::printValue_impl(obj, (short)0);
+  }
+
+  // Arrays
+  template<typename T, size_t N>
+  std::string printValue(const T (*obj)[N]) {
+    if (N == 0)
+      return valuePrinterInternal::kEmptyCollection;
+
+    std::string str = "{ ";
+    str += printValue(*obj + 0);
+    for (size_t i = 1; i < N; ++i) {
+      str += ", ";
+      str += printValue(*obj + i);
+    }
+    return str + " }";
   }
 
   // Tuples
@@ -269,6 +247,8 @@ namespace cling {
   std::string printValue(std::tuple<ARGS...> *val)
   {
     using T = std::tuple<ARGS...>;
+    if (std::tuple_size<T>::value == 0)
+      return valuePrinterInternal::kEmptyCollection;
     return collectionPrinterInternal::tuplePairPrintValue<T>(val);
   }
 

lib/Interpreter/ValuePrinter.cpp

@@ -44,6 +44,13 @@ extern "C" void cling_PrintValue(void * /*cling::Value**/ V) {
   //std::string valueStr = printValueInternal(*value);
 }
 
+// Exported for RuntimePrintValue.h
+namespace cling {
+  namespace valuePrinterInternal {
+    extern const char* const kEmptyCollection = "{}";
+  }
+}
+
 namespace {
 
 static std::string enclose(const std::string& Mid, const char* Begin,

test/Prompt/ValuePrinter/Collections.C

@@ -0,0 +1,38 @@
+//------------------------------------------------------------------------------
+// CLING - the C++ LLVM-based InterpreterG :)
+//
+// This file is dual-licensed: you can choose to license it under the University
+// of Illinois Open Source License or the GNU Lesser General Public License. See
+// LICENSE.TXT for details.
+
+//------------------------------------------------------------------------------
+
+// RUN: cat %s | %cling -Xclang -verify 2>&1 | FileCheck %s
+
+#include <string>
+#include <tuple>
+#include <vector>
+
+std::vector<bool> Bv(5,5)
+// FIXME: Printing std::vector<bool> is still broken.
+// But the line above at least tests cling doesn't crash because of it.
+// BROKENCHECK: (std::vector<bool> &) { true, true, true, true, true }
+
+class CustomThing {
+};
+
+namespace cling {
+  std::string printValue(const CustomThing *ptr) {
+    return "";
+  }
+}
+
+std::vector<CustomThing> A, B(1);
+cling::printValue(&A) == cling::printValue(&B)
+// CHECK: (bool) false
+
+std::tuple<> tA
+// CHECK: (std::tuple<> &) {}
+
+// expected-no-diagnostics
+.q

test/Prompt/ValuePrinter/TuplesAndPairs.C

@@ -20,7 +20,7 @@ std::make_pair(4L,'c')
 //CHECK: (std::pair<{{.*long.*,.*char.*}}>) { 4, 'c' }
 
 std::make_tuple()
-//CHECK: (std::tuple<>) {  }
+//CHECK: (std::tuple<>) {}
 
 std::make_tuple(2)
 //CHECK: (std::tuple<{{.*int.*}}>) { 2 }

tools/driver/CMakeLists.txt

@@ -39,6 +39,8 @@ set_target_properties(cling
 if(MSVC)
   set_target_properties(cling
     PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS 1)
+  set_property(TARGET cling APPEND_STRING PROPERTY LINK_FLAGS
+    " /EXPORT:?kEmptyCollection@valuePrinterInternal@cling@@3QEBDEB ")
 endif(MSVC)
 
 target_link_libraries(cling