X-Git-Url: http://plrg.eecs.uci.edu/git/?a=blobdiff_plain;f=folly%2FFormat-inl.h;h=5331bbba199038caed2cae4e9104ebcb40d5fe1e;hb=8b05be27769e2e3d5d1681251953506c22f76b45;hp=620557a6b435a80b139b241975952f9545d8dafb;hpb=9bfa4150776b1373827cd1104de852ec91350376;p=folly.git

diff --git a/folly/Format-inl.h b/folly/Format-inl.h
index 620557a6..5331bbba 100644
--- a/folly/Format-inl.h
+++ b/folly/Format-inl.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 Facebook, Inc.
+ * Copyright 2014 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,7 +18,12 @@
 #error This file may only be included from Format.h.
 #endif
 
-#include "folly/Traits.h"
+#include <folly/Exception.h>
+#include <folly/Traits.h>
+
+// Ignore -Wformat-nonliteral warnings within this file
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
 
 namespace folly {
 
@@ -136,19 +141,64 @@ size_t uintToBinary(char* buffer, size_t bufLen, Uint v) {
 
 }  // namespace detail
 
-
-template <bool containerMode, class... Args>
-Formatter<containerMode, Args...>::Formatter(StringPiece str, Args&&... args)
-  : str_(str),
-    values_(FormatValue<typename std::decay<Args>::type>(
-        std::forward<Args>(args))...) {
+template <class Derived, bool containerMode, class... Args>
+BaseFormatter<Derived, containerMode, Args...>::BaseFormatter(StringPiece str,
+                                                              Args&&... args)
+    : str_(str),
+      values_(FormatValue<typename std::decay<Args>::type>(
+          std::forward<Args>(args))...) {
   static_assert(!containerMode || sizeof...(Args) == 1,
                 "Exactly one argument required in container mode");
 }
 
-template <bool containerMode, class... Args>
+template <class Derived, bool containerMode, class... Args>
+void BaseFormatter<Derived, containerMode, Args...>::handleFormatStrError()
+    const {
+  if (crashOnError_) {
+    LOG(FATAL) << "folly::format: bad format string \"" << str_ << "\": " <<
+      folly::exceptionStr(std::current_exception());
+  }
+  throw;
+}
+
+template <class Derived, bool containerMode, class... Args>
 template <class Output>
-void Formatter<containerMode, Args...>::operator()(Output& out) const {
+void BaseFormatter<Derived, containerMode, Args...>::operator()(Output& out)
+    const {
+  // Catch BadFormatArg and range_error exceptions, and call
+  // handleFormatStrError().
+  //
+  // These exception types indicate a problem with the format string.  Most
+  // format strings are string literals specified by the programmer.  If they
+  // have a problem, this is usually a programmer bug.  We want to crash to
+  // ensure that these are found early on during development.
+  //
+  // BadFormatArg is thrown by the Format.h code, while range_error is thrown
+  // by Conv.h, which is used in several places in our format string
+  // processing.
+  //
+  // (Note: This behavior is slightly dangerous.  If the Output object throws a
+  // BadFormatArg or a range_error, we will also crash the program, even if it
+  // wasn't an issue with the format string.  This seems highly unlikely
+  // though, and none of our current Output objects can throw these errors.)
+  //
+  // We also throw out_of_range errors if the format string references an
+  // argument that isn't present (or a key that isn't present in one of the
+  // argument containers).  However, at the moment we don't crash on these
+  // errors, as it is likely that the container is dynamic at runtime.
+  try {
+    appendOutput(out);
+  } catch (const BadFormatArg& ex) {
+    handleFormatStrError();
+  } catch (const std::range_error& ex) {
+    handleFormatStrError();
+  }
+}
+
+template <class Derived, bool containerMode, class... Args>
+template <class Output>
+void BaseFormatter<Derived, containerMode, Args...>::appendOutput(Output& out)
+    const {
   auto p = str_.begin();
   auto end = str_.end();
 
@@ -168,7 +218,9 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
       out(StringPiece(p, q));
       p = q;
 
-      CHECK(p != end && *p == '}') << "single '}' in format string";
+      if (p == end || *p != '}') {
+        throw BadFormatArg("folly::format: single '}' in format string");
+      }
       ++p;
     }
   };
@@ -185,7 +237,9 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
     outputString(StringPiece(p, q));
     p = q + 1;
 
-    CHECK(p != end) << "'{' at end of format string";
+    if (p == end) {
+      throw BadFormatArg("folly::format: '}' at end of format string");
+    }
 
     // "{{" -> "{"
     if (*p == '{') {
@@ -196,7 +250,9 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
 
     // Format string
     q = static_cast<const char*>(memchr(p, '}', end - p));
-    CHECK(q != end) << "missing ending '}'";
+    if (q == nullptr) {
+      throw BadFormatArg("folly::format: missing ending '}'");
+    }
     FormatArg arg(StringPiece(p, q));
     p = q + 1;
 
@@ -218,27 +274,51 @@ void Formatter<containerMode, Args...>::operator()(Output& out) const {
         try {
           argIndex = to<int>(piece);
         } catch (const std::out_of_range& e) {
-          LOG(FATAL) << "argument index must be integer";
+          arg.error("argument index must be integer");
         }
-        CHECK(argIndex >= 0)
-          << arg.errorStr("argument index must be non-negative");
+        arg.enforce(argIndex >= 0, "argument index must be non-negative");
         hasExplicitArgIndex = true;
       }
     }
 
-    CHECK(!hasDefaultArgIndex || !hasExplicitArgIndex)
-      << "may not have both default and explicit arg indexes";
+    if (hasDefaultArgIndex && hasExplicitArgIndex) {
+      throw BadFormatArg(
+          "folly::format: may not have both default and explicit arg indexes");
+    }
 
     doFormat(argIndex, arg, out);
   }
 }
 
+template <class Derived, bool containerMode, class... Args>
+void writeTo(FILE* fp,
+             const BaseFormatter<Derived, containerMode, Args...>& formatter) {
+  auto writer = [fp] (StringPiece sp) {
+    ssize_t n = fwrite(sp.data(), 1, sp.size(), fp);
+    if (n < sp.size()) {
+      throwSystemError("Formatter writeTo", "fwrite failed");
+    }
+  };
+  formatter(writer);
+}
+
 namespace format_value {
 
 template <class FormatCallback>
 void formatString(StringPiece val, FormatArg& arg, FormatCallback& cb) {
+  if (arg.width != FormatArg::kDefaultWidth && arg.width < 0) {
+    throw BadFormatArg("folly::format: invalid width");
+  }
+  if (arg.precision != FormatArg::kDefaultPrecision && arg.precision < 0) {
+    throw BadFormatArg("folly::format: invalid precision");
+  }
+
+  // XXX: clang should be smart enough to not need the two static_cast<size_t>
+  // uses below given the above checks. If clang ever becomes that smart, we
+  // should remove the otherwise unnecessary warts.
+
   if (arg.precision != FormatArg::kDefaultPrecision &&
-      val.size() > arg.precision) {
+      val.size() > static_cast<size_t>(arg.precision)) {
     val.reset(val.data(), arg.precision);
   }
 
@@ -255,9 +335,10 @@ void formatString(StringPiece val, FormatArg& arg, FormatCallback& cb) {
   };
 
   int padRemaining = 0;
-  if (arg.width != FormatArg::kDefaultWidth && val.size() < arg.width) {
+  if (arg.width != FormatArg::kDefaultWidth &&
+      val.size() < static_cast<size_t>(arg.width)) {
     char fill = arg.fill == FormatArg::kDefaultFill ? ' ' : arg.fill;
-    int padChars = arg.width - val.size();
+    int padChars = static_cast<int> (arg.width - val.size());
     memset(padBuf, fill, std::min(padBufSize, padChars));
 
     switch (arg.align) {
@@ -302,10 +383,14 @@ void formatNumber(StringPiece val, int prefixLen, FormatArg& arg,
   format_value::formatString(val, arg, cb);
 }
 
-template <class FormatCallback, bool containerMode, class... Args>
-void formatFormatter(const Formatter<containerMode, Args...>& formatter,
-                     FormatArg& arg,
-                     FormatCallback& cb) {
+template <class FormatCallback,
+          class Derived,
+          bool containerMode,
+          class... Args>
+void formatFormatter(
+    const BaseFormatter<Derived, containerMode, Args...>& formatter,
+    FormatArg& arg,
+    FormatCallback& cb) {
   if (arg.width == FormatArg::kDefaultWidth &&
       arg.precision == FormatArg::kDefaultPrecision) {
     // nothing to do
@@ -391,8 +476,8 @@ class FormatValue<
       uval = val_;
       sign = '\0';
 
-      CHECK(arg.sign == FormatArg::Sign::DEFAULT)
-        << arg.errorStr("sign specifications not allowed for unsigned values");
+      arg.enforce(arg.sign == FormatArg::Sign::DEFAULT,
+                  "sign specifications not allowed for unsigned values");
     }
 
     // max of:
@@ -419,9 +504,9 @@ class FormatValue<
     switch (presentation) {
     case 'n':  // TODO(tudorb): locale awareness?
     case 'd':
-      CHECK(!arg.basePrefix)
-        << arg.errorStr("base prefix not allowed with '", presentation,
-                        "' specifier");
+      arg.enforce(!arg.basePrefix,
+                  "base prefix not allowed with '", presentation,
+                  "' specifier");
       if (arg.thousandsSeparator) {
         useSprintf("%'ju");
       } else {
@@ -431,21 +516,21 @@ class FormatValue<
       }
       break;
     case 'c':
-      CHECK(!arg.basePrefix)
-        << arg.errorStr("base prefix not allowed with '", presentation,
-                        "' specifier");
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.basePrefix,
+                  "base prefix not allowed with '", presentation,
+                  "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");
       valBufBegin = valBuf + 3;
       *valBufBegin = static_cast<char>(uval);
       valBufEnd = valBufBegin + 1;
       break;
     case 'o':
     case 'O':
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToOctal(valBuf, valBufSize - 1, uval);
       if (arg.basePrefix) {
@@ -454,9 +539,9 @@ class FormatValue<
       }
       break;
     case 'x':
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexLower(valBuf, valBufSize - 1,
                                                     uval);
@@ -467,9 +552,9 @@ class FormatValue<
       }
       break;
     case 'X':
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToHexUpper(valBuf, valBufSize - 1,
                                                     uval);
@@ -481,9 +566,9 @@ class FormatValue<
       break;
     case 'b':
     case 'B':
-      CHECK(!arg.thousandsSeparator)
-        << arg.errorStr("thousands separator (',') not allowed with '",
-                        presentation, "' specifier");
+      arg.enforce(!arg.thousandsSeparator,
+                  "thousands separator (',') not allowed with '",
+                  presentation, "' specifier");
       valBufEnd = valBuf + valBufSize - 1;
       valBufBegin = valBuf + detail::uintToBinary(valBuf, valBufSize - 1,
                                                   uval);
@@ -494,7 +579,7 @@ class FormatValue<
       }
       break;
     default:
-      LOG(FATAL) << arg.errorStr("invalid specifier '", presentation, "'");
+      arg.error("invalid specifier '", presentation, "'");
     }
 
     if (sign) {
@@ -561,7 +646,7 @@ class FormatValue<double> {
          DoubleToStringConverter::kMaxFixedDigitsAfterPoint),
         (8 + DoubleToStringConverter::kMaxExponentialDigits),
         (7 + DoubleToStringConverter::kMaxPrecisionDigits)})];
-    StringBuilder builder(buf + 1, sizeof(buf) - 1);
+    StringBuilder builder(buf + 1, static_cast<int> (sizeof(buf) - 1));
 
     char plusSign;
     switch (arg.sign) {
@@ -576,6 +661,11 @@ class FormatValue<double> {
       break;
     };
 
+    auto flags =
+        DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN |
+        (arg.trailingDot ? DoubleToStringConverter::EMIT_TRAILING_DECIMAL_POINT
+                         : 0);
+
     double val = val_;
     switch (arg.presentation) {
     case '%':
@@ -587,13 +677,14 @@ class FormatValue<double> {
             DoubleToStringConverter::kMaxFixedDigitsAfterPoint) {
           arg.precision = DoubleToStringConverter::kMaxFixedDigitsAfterPoint;
         }
-        DoubleToStringConverter conv(
-            DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN,
-            infinitySymbol,
-            nanSymbol,
-            exponentSymbol,
-            -4, arg.precision,
-            0, 0);
+        DoubleToStringConverter conv(flags,
+                                     infinitySymbol,
+                                     nanSymbol,
+                                     exponentSymbol,
+                                     -4,
+                                     arg.precision,
+                                     0,
+                                     0);
         arg.enforce(conv.ToFixed(val, arg.precision, &builder),
                     "fixed double conversion failed");
       }
@@ -605,14 +696,15 @@ class FormatValue<double> {
           arg.precision = DoubleToStringConverter::kMaxExponentialDigits;
         }
 
-        DoubleToStringConverter conv(
-            DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN,
-            infinitySymbol,
-            nanSymbol,
-            exponentSymbol,
-            -4, arg.precision,
-            0, 0);
-        CHECK(conv.ToExponential(val, arg.precision, &builder));
+        DoubleToStringConverter conv(flags,
+                                     infinitySymbol,
+                                     nanSymbol,
+                                     exponentSymbol,
+                                     -4,
+                                     arg.precision,
+                                     0,
+                                     0);
+        arg.enforce(conv.ToExponential(val, arg.precision, &builder));
       }
       break;
     case 'n':  // should be locale-aware, but isn't
@@ -625,18 +717,19 @@ class FormatValue<double> {
                    DoubleToStringConverter::kMaxPrecisionDigits) {
           arg.precision = DoubleToStringConverter::kMaxPrecisionDigits;
         }
-        DoubleToStringConverter conv(
-            DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN,
-            infinitySymbol,
-            nanSymbol,
-            exponentSymbol,
-            -4, arg.precision,
-            0, 0);
-        CHECK(conv.ToShortest(val, &builder));
+        DoubleToStringConverter conv(flags,
+                                     infinitySymbol,
+                                     nanSymbol,
+                                     exponentSymbol,
+                                     -4,
+                                     arg.precision,
+                                     0,
+                                     0);
+        arg.enforce(conv.ToShortest(val, &builder));
       }
       break;
     default:
-      LOG(FATAL) << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.error("invalid specifier '", arg.presentation, "'");
     }
 
     int len = builder.position();
@@ -693,9 +786,9 @@ class FormatValue<
   void format(FormatArg& arg, FormatCallback& cb) const {
     if (arg.keyEmpty()) {
       arg.validate(FormatArg::Type::OTHER);
-      CHECK(arg.presentation == FormatArg::kDefaultPresentation ||
-            arg.presentation == 's')
-        << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.enforce(arg.presentation == FormatArg::kDefaultPresentation ||
+                  arg.presentation == 's',
+                  "invalid specifier '", arg.presentation, "'");
       format_value::formatString(val_, arg, cb);
     } else {
       FormatValue<char>(val_.at(arg.splitIntKey())).format(arg, cb);
@@ -715,8 +808,8 @@ class FormatValue<std::nullptr_t> {
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     arg.validate(FormatArg::Type::OTHER);
-    CHECK(arg.presentation == FormatArg::kDefaultPresentation)
-      << arg.errorStr("invalid specifier '", arg.presentation, "'");
+    arg.enforce(arg.presentation == FormatArg::kDefaultPresentation,
+                "invalid specifier '", arg.presentation, "'");
     format_value::formatString("(null)", arg, cb);
   }
 };
@@ -766,8 +859,8 @@ class FormatValue<
     } else {
       // Print as a pointer, in hex.
       arg.validate(FormatArg::Type::OTHER);
-      CHECK(arg.presentation == FormatArg::kDefaultPresentation)
-        << arg.errorStr("invalid specifier '", arg.presentation, "'");
+      arg.enforce(arg.presentation == FormatArg::kDefaultPresentation,
+                  "invalid specifier '", arg.presentation, "'");
       arg.basePrefix = true;
       arg.presentation = 'x';
       if (arg.align == FormatArg::Align::DEFAULT) {
@@ -787,7 +880,7 @@ class TryFormatValue {
  public:
   template <class FormatCallback>
   static void formatOrFail(T& value, FormatArg& arg, FormatCallback& cb) {
-    LOG(FATAL) << arg.errorStr("No formatter available for this type");
+    arg.error("No formatter available for this type");
   }
 };
 
@@ -847,6 +940,11 @@ struct IndexableTraitsSeq : public FormatTraitsBase {
   static const value_type& at(const C& c, int idx) {
     return c.at(idx);
   }
+
+  static const value_type& at(const C& c, int idx,
+                              const value_type& dflt) {
+    return (idx >= 0 && idx < c.size()) ? c.at(idx) : dflt;
+  }
 };
 
 // Base class for associative types (maps)
@@ -856,6 +954,11 @@ struct IndexableTraitsAssoc : public FormatTraitsBase {
   static const value_type& at(const C& c, int idx) {
     return c.at(static_cast<typename C::key_type>(idx));
   }
+  static const value_type& at(const C& c, int idx,
+                              const value_type& dflt) {
+    auto pos = c.find(static_cast<typename C::key_type>(idx));
+    return pos != c.end() ? pos->second : dflt;
+  }
 };
 
 // std::array
@@ -926,6 +1029,28 @@ class FormatValue<
   const T& val_;
 };
 
+template <class Container, class Value>
+class FormatValue<
+  detail::DefaultValueWrapper<Container, Value>,
+  typename detail::IndexableTraits<Container>::enabled> {
+ public:
+  explicit FormatValue(const detail::DefaultValueWrapper<Container, Value>& val)
+    : val_(val) { }
+
+  template <class FormatCallback>
+  void format(FormatArg& arg, FormatCallback& cb) const {
+    FormatValue<typename std::decay<
+      typename detail::IndexableTraits<Container>::value_type>::type>(
+          detail::IndexableTraits<Container>::at(
+              val_.container,
+              arg.splitIntKey(),
+              val_.defaultValue)).format(arg, cb);
+  }
+
+ private:
+  const detail::DefaultValueWrapper<Container, Value>& val_;
+};
+
 namespace detail {
 
 // Define enabled, key_type, convert from StringPiece to the key types
@@ -967,6 +1092,11 @@ template <class T> struct KeyableTraitsAssoc : public FormatTraitsBase {
   static const value_type& at(const T& map, StringPiece key) {
     return map.at(KeyFromStringPiece<key_type>::convert(key));
   }
+  static const value_type& at(const T& map, StringPiece key,
+                              const value_type& dflt) {
+    auto pos = map.find(KeyFromStringPiece<key_type>::convert(key));
+    return pos != map.end() ? pos->second : dflt;
+  }
 };
 
 // Define enabled, key_type, value_type, at() for supported string-keyed
@@ -1011,6 +1141,28 @@ class FormatValue<
   const T& val_;
 };
 
+template <class Container, class Value>
+class FormatValue<
+  detail::DefaultValueWrapper<Container, Value>,
+  typename detail::KeyableTraits<Container>::enabled> {
+ public:
+  explicit FormatValue(const detail::DefaultValueWrapper<Container, Value>& val)
+    : val_(val) { }
+
+  template <class FormatCallback>
+  void format(FormatArg& arg, FormatCallback& cb) const {
+    FormatValue<typename std::decay<
+      typename detail::KeyableTraits<Container>::value_type>::type>(
+          detail::KeyableTraits<Container>::at(
+              val_.container,
+              arg.splitKey(),
+              val_.defaultValue)).format(arg, cb);
+  }
+
+ private:
+  const detail::DefaultValueWrapper<Container, Value>& val_;
+};
+
 // Partial specialization of FormatValue for pairs
 template <class A, class B>
 class FormatValue<std::pair<A, B>> {
@@ -1028,7 +1180,7 @@ class FormatValue<std::pair<A, B>> {
       FormatValue<typename std::decay<B>::type>(val_.second).format(arg, cb);
       break;
     default:
-      LOG(FATAL) << arg.errorStr("invalid index for pair");
+      arg.error("invalid index for pair");
     }
   }
 
@@ -1046,7 +1198,7 @@ class FormatValue<std::tuple<Args...>> {
   template <class FormatCallback>
   void format(FormatArg& arg, FormatCallback& cb) const {
     int key = arg.splitIntKey();
-    CHECK(key >= 0) << arg.errorStr("tuple index must be non-negative");
+    arg.enforce(key >= 0, "tuple index must be non-negative");
     doFormat(key, arg, cb);
   }
 
@@ -1056,7 +1208,7 @@ class FormatValue<std::tuple<Args...>> {
   template <size_t K, class Callback>
   typename std::enable_if<K == valueCount>::type
   doFormatFrom(size_t i, FormatArg& arg, Callback& cb) const {
-    LOG(FATAL) << arg.errorStr("tuple index out of range, max=", i);
+    arg.enforce("tuple index out of range, max=", i);
   }
 
   template <size_t K, class Callback>
@@ -1080,9 +1232,13 @@ class FormatValue<std::tuple<Args...>> {
 };
 
 // Partial specialization of FormatValue for nested Formatters
-template <bool containerMode, class... Args>
-class FormatValue<Formatter<containerMode, Args...>, void> {
-  typedef Formatter<containerMode, Args...> FormatterValue;
+template <bool containerMode, class... Args,
+          template <bool, class...> class F>
+class FormatValue<F<containerMode, Args...>,
+                  typename std::enable_if<detail::IsFormatter<
+                      F<containerMode, Args...>>::value>::type> {
+  typedef typename F<containerMode, Args...>::BaseType FormatterValue;
+
  public:
   explicit FormatValue(const FormatterValue& f) : f_(f) { }
 
@@ -1098,11 +1254,12 @@ class FormatValue<Formatter<containerMode, Args...>, void> {
  * Formatter objects can be appended to strings, and therefore they're
  * compatible with folly::toAppend and folly::to.
  */
-template <class Tgt, bool containerMode, class... Args>
-typename std::enable_if<
-   detail::IsSomeString<Tgt>::value>::type
-toAppend(const Formatter<containerMode, Args...>& value, Tgt * result) {
+template <class Tgt, class Derived, bool containerMode, class... Args>
+typename std::enable_if<IsSomeString<Tgt>::value>::type toAppend(
+    const BaseFormatter<Derived, containerMode, Args...>& value, Tgt* result) {
   value.appendTo(*result);
 }
 
 }  // namespace folly
+
+#pragma GCC diagnostic pop