[analyzer] print() JSONify: Store implementation

[Charusso]

Summary

-

[NoQ]

Yay great!

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

128 ↗

(On Diff #199491)

Let's make the caller responsible for this last comma.

2623 ↗

(On Diff #199491)

Let's turn this whole thing into a valid JSON, i.e., say, { "title": "Store", "items": ... }.

[NoQ]

clang/include/clang/Basic/JsonSupport.h

17–21 ↗

(On Diff #199491)

Sigh, that's roughly the ~5th implementation of this functionality. I also don't think there is going to be anything JSON-specific here. That said, i don't have an immediate better suggestion on where to put it.

[Charusso]

• More JSONify.

[Charusso]

PrettyStackTraceLocationContext.h definitely needs the default print() method (may other users too). Even it is a little bit silly but I think we have to keep them and create printJSON(). What do you think?

clang/include/clang/Basic/JsonSupport.h

17–21 ↗

(On Diff #199491)

I think the CTU is will be one of the users here and may we too with HTML parsing. This place is all about spacing and new-lines.

[Charusso]

clang/include/clang/Basic/JsonSupport.h

21 ↗

(On Diff #199667)

E.g. something like that would be very helpful, but I will remove that by the next patch.

[NoQ]

In D61912#1503542, @Charusso wrote:

PrettyStackTraceLocationContext.h definitely needs the default print() method (may other users too). Even it is a little bit silly but I think we have to keep them and create printJSON(). What do you think?

Yeah, we'll have to inevitably duplicate this one, i guess. I'm not really sure how important our pretty stack traces are, but i guess let's not break them.

[NoQ]

Generally this is good to go, i think. I like it!

clang/test/Analysis/expr-inspection.c

15 ↗

(On Diff #199667)

<3

[Charusso]

• Better header handling.
• Less JSON.

[Charusso]

Thanks for the accept! Sorry for the extra noise but it came out it is tricky to create valid JSON for each sub-print and we do not need it for now. I wanted to create the most compact and necessary JSON without touching the JsonSupport.h.

clang/test/Analysis/expr-inspection.c

15 ↗

(On Diff #199667)

If you have mentioned it, I would bet you do not <3 easter eggs. Reverted being a funny kid.

[Charusso]

PS: Outcome of program state printing could be found in D62087 which is very similar to your idea in D61677.

[NoQ]

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

2627 ↗

(On Diff #200096)

I think mentioning Store twice is redundant. I'll be perfectly happy with either

"program_state": [
  { "title": "Store", "items": [ ... ]},
  { "title": "Environment", "items": [ ... ]},
  ...
]

or

"program_state": [
  "store": [ ... ],
  "environment": [ ... ],
  ...
]

clang/test/Analysis/expr-inspection.c

15 ↗

(On Diff #199667)

^.^

[Charusso]

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

2627 ↗

(On Diff #200096)

You have to navigate inside the JSON tree so the second part is necessary, but the first part is the title-idea to put some info as a title. E.g. [Store (ID 666, Type: Region)] and do not create extra JSON-field where it does not need to be handled in HTML, nor CTU. It happens mostly with IDs.

[Charusso]

After a while I think we could drop the IDs and the title part.

Here is a plan 2.0:

• Drop every ID.
• If any of the Store, Environment, etc. has not got any element make it null, so that drop the items part with the title.

This is the graph which is the list of nodes similar to ExplodedGraph:

{ graph: [
...
{ id: 13, "predecessors": [12], "successors": [14, 15],
  "program_points": [
    ...
  ],
  "program_state": {
    "store": null,
    "environment": [
      { "location_context": ... },
    ], ...
  }
},
{ id: 14, ...
  ...
},
...
]}

There is a concept of an empty array ("store": [],), but I think it is more precise if it is null instead.

[Charusso]

Another plan: Because our representation will be pure JSON the Sep parameter will be useless, so I remove it.

[NoQ]

In D61912#1509693, @Charusso wrote:

After a while I think we could drop the IDs and the title part.

I'm using IDs, don't drop them^^ They are useful when you're viewing ExplodedGraph from a debugger: with IDs you can either seek the current node in the graph, or, more importantly, set conditional breakpoints with IDs as conditions.

In D61912#1509693, @Charusso wrote:

Here is a plan 2.0:

There isn't much time left, so i think we should start "converging", i.e. produce something usable and stable, and then later follow up with further updates if necessary. It isn't worth it to perfect the format until we see how it works in practice.

In D61912#1510539, @Charusso wrote:

Another plan: Because our representation will be pure JSON the Sep parameter will be useless, so I remove it.

You can't do that unless you remove the .dot dumper entirely, because for .dot files Sep has to be \\l. For now it's not done, so let's delay it.

[Charusso]

In D61912#1512671, @NoQ wrote:

In D61912#1509693, @Charusso wrote:

After a while I think we could drop the IDs and the title part.

I'm using IDs, don't drop them^^ They are useful when you're viewing ExplodedGraph from a debugger: with IDs you can either seek the current node in the graph, or, more importantly, set conditional breakpoints with IDs as conditions.

I meant the [Store (ID)] title business and the pointers which appears between < > like the ID of the Store.

If we write out some ProgramPoints it looks like that:

DeclRefExpr S2079258 <0x29c35c0> RM
line=2715 col=11
--------
ImplicitCastExpr S2079262 <0x29c35e0> RM
line=2715 col=11

Would you need the S or the < > version of IDs, or both? How would you call the latter?

In D61912#1509693, @Charusso wrote:

Here is a plan 2.0:

There isn't much time left, so i think we should start "converging", i.e. produce something usable and stable, and then later follow up with further updates if necessary. It isn't worth it to perfect the format until we see how it works in practice.

I have planned this just today, so I will rewrite everything properly within the next 24h, do not worry about timings yet. I just wanted to see what would you inject or remove from the current ProgramState representation, that is why I have requested a review for each of its descendants. The ProgramPoint business is much simpler. The node pred and succ gathering is useful for creating a very smart JSON graph which is at most 30 min work with full review.

In D61912#1510539, @Charusso wrote:

Another plan: Because our representation will be pure JSON the Sep parameter will be useless, so I remove it.

You can't do that unless you remove the .dot dumper entirely, because for .dot files Sep has to be \\l. For now it's not done, so let's delay it.

I will not remove the DOT format, just the separation will be useless as every node will be 100% pure JSON and the spacing will be the format itself.

However, the DOT-dump is broken because the spaces going to be trimmed which is fail. Because the JSON format it is not that big deal, here is a crazy example from the old version:

{ "title": "Environment", "items": [
{ "location_context": "#0 Call", "calling": "llvm::APInt::operator==", "call_line": "1743", "items": [
{ "lctx_id": 375, "stmt_id": 845291, "pretty": "this->BitWidth == RHS.BitWidth", "value": "1 U1b" },
{ "lctx_id": 375, "stmt_id": 845306, "pretty": ""Comparison requires equal bit widths"", "value": "1 U1b" },
]},
{ "location_context": "#1 Call", "calling": "llvm::APInt::udivrem", "call_line": "1847", "items": [
{ "lctx_id": 318, "stmt_id": 2048059, "pretty": "LHS", "value": "&SymRegion{reg_$1<const class llvm::APInt & A>}" },
{ "lctx_id": 318, "stmt_id": 2048063, "pretty": "RHS", "value": "&temp_object{class llvm::APInt, S2050862}" },
{ "lctx_id": 318, "stmt_id": 2048107, "pretty": "operator==", "value": "&code{operator==}" },
]},
]}

I have not seen any way to bypass this behaviour sadly.

[NoQ]

In D61912#1512708, @Charusso wrote:

I meant the [Store (ID)] title business and the pointers which appears between < > like the ID of the Store.

If we write out some ProgramPoints it looks like that:

DeclRefExpr S2079258 <0x29c35c0> RM
line=2715 col=11
--------
ImplicitCastExpr S2079262 <0x29c35e0> RM
line=2715 col=11

Would you need the S or the < > version of IDs, or both? How would you call the latter?

I use both. Let's call the latter "pointer" because it's literally a pointer; you can dereference it within a debugger if you're within the same process:

p ((DeclRefExpr *)0x29c35c0)->dump()

I just wanted to see what would you inject or remove from the current ProgramState representation, that is why I have requested a review for each of its descendants.

I'd rather not change anything for now. I'm using almost all of the information that's currently present in the graph.

I will not remove the DOT format, just the separation will be useless as every node will be 100% pure JSON and the spacing will be the format itself.

Emm, right, Sep is indeed useless, NL is not useless, sry^^

However, the DOT-dump is broken because the spaces going to be trimmed which is fail.
...
I have not seen any way to bypass this behaviour sadly.

Did you try indenting with &nbsp;?

[Charusso]

• Improved the JSON format.

[Charusso]

Thanks @NoQ! The &nbsp; works well, but it produces a huge SVG file, here is a complete example of node-printing:

node-printing.svg18 MBDownload

[NoQ]

In D61912#1514696, @Charusso wrote:

Thanks @NoQ! The &nbsp; works well, but it produces a huge SVG file, here is a complete example of node-printing:

node-printing.svg18 MBDownload

Looks fantastic. I can work with that.
I hope that we'll be able to remove the &nbsp; thing soon, as we go for a fancier viewer.

[Charusso]

• 100% pure JSON.

[lebedev.ri]

All these patches bypassed cfe-commits.

Why does this invent a yet another json formatter instead of using
"llvm/Support/JSON.h", in particular it's lightweight json::OStream ?

[Charusso]

Hey @lebedev.ri, thanks for the review!

In D61912#1521306, @lebedev.ri wrote:

All these patches bypassed cfe-commits.

Bypassed? It is only added when you push your stuff, which is happened as expected.

Why does this invent a yet another json formatter instead of using
"llvm/Support/JSON.h", in particular it's lightweight json::OStream ?

We are doing our own JSON representation which is not really the job of json namespace. Also it is my personal feeling to avoid write::like::that for perfectly no reason.

[lebedev.ri]

In D61912#1521352, @Charusso wrote:

Hey @lebedev.ri, thanks for the review!

In D61912#1521306, @lebedev.ri wrote:

All these patches bypassed cfe-commits.

Bypassed? It is only added when you push your stuff, which is happened as expected.

That is pretty much the opposite from what is exected, e.g.:
https://llvm.org/docs/Phabricator.html

While Phabricator is a useful tool for some, the relevant -commits mailing list
is the system of record for all LLVM code review. The mailing list should be added
as a subscriber on all reviews, and Phabricator users should be prepared to respond
to free-form comments in mail sent to the commits list.

Why does this invent a yet another json formatter instead of using
"llvm/Support/JSON.h", in particular it's lightweight json::OStream ?

We are doing our own JSON representation which is not really the job of json namespace. Also it is my personal feeling to avoid write::like::that for perfectly no reason.

That was precisely the question. Why is this not using the abstractions, but does everything on it's own?

[Charusso]

In D61912#1521306, @lebedev.ri wrote:

All these patches bypassed cfe-commits.

Bypassed? It is only added when you push your stuff, which is happened as expected.

That is pretty much the opposite from what is exected, e.g.:
https://llvm.org/docs/Phabricator.html

While Phabricator is a useful tool for some, the relevant -commits mailing list
is the system of record for all LLVM code review. The mailing list should be added
as a subscriber on all reviews, and Phabricator users should be prepared to respond
to free-form comments in mail sent to the commits list.

Hm, in my mind it was in the proper order, as you could see that. Thanks for the clarification! I will add it.

Why does this invent a yet another json formatter instead of using
"llvm/Support/JSON.h", in particular it's lightweight json::OStream ?

We are doing our own JSON representation which is not really the job of json namespace. Also it is my personal feeling to avoid write::like::that for perfectly no reason.

That was precisely the question. Why is this not using the abstractions, but does everything on it's own?

There is that much information per node of the ExplodedGraph that cannot fit into a 8K monitor screen but you have to see the whole node and its predecessor to understand it. We are trying to compact and reduce it as small as possible. At the moment our representation is a little bit wider than it should be. Here is an ugly WIP example: D62553

[Szelethus]

In D61912#1521411, @Charusso wrote:

In D61912#1521306, @lebedev.ri wrote:

All these patches bypassed cfe-commits.

Bypassed? It is only added when you push your stuff, which is happened as expected.

That is pretty much the opposite from what is exected, e.g.:
https://llvm.org/docs/Phabricator.html

While Phabricator is a useful tool for some, the relevant -commits mailing list
is the system of record for all LLVM code review. The mailing list should be added
as a subscriber on all reviews, and Phabricator users should be prepared to respond
to free-form comments in mail sent to the commits list.

Hm, in my mind it was in the proper order, as you could see that. Thanks for the clarification! I will add it.

I think cfe-commits is added automatically if you add "Clang" to the "Repository" field when creating a new Differential.

[Charusso]

In D61912#1521476, @Szelethus wrote:

I think cfe-commits is added automatically if you add "Clang" to the "Repository" field when creating a new Differential.

Exactly, thanks!

include/clang/Basic/JsonSupport.h

+//===- JsonSupport.h - JSON Output Utilities --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_BASIC_JSONSUPPORT_H
+#define LLVM_CLANG_BASIC_JSONSUPPORT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/Support/raw_ostream.h"
+
+
+namespace clang {
+
+inline raw_ostream &Indent(raw_ostream &Out, const unsigned int Space,
+                           bool IsDot) {
+  for (unsigned int I = 0; I < Space * 2; ++I)
+    Out << (IsDot ? "&nbsp;" : " ");
+  return Out;
+}
+
+} // namespace clang
+
+#endif // LLVM_CLANG_BASIC_JSONSUPPORT_H

include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h

public:
 
   template<typename T>
   bool contains(typename ProgramStateTrait<T>::key_type key) const {
     void *const* d = FindGDM(ProgramStateTrait<T>::GDMIndex());
     return ProgramStateTrait<T>::Contains(ProgramStateTrait<T>::MakeData(d), key);
   }
 
   // Pretty-printing.
-  void print(raw_ostream &Out, const char *nl = "\n", const char *sep = "",
-             const LocationContext *CurrentLC = nullptr) const;
-  void printDOT(raw_ostream &Out,
-                const LocationContext *CurrentLC = nullptr) const;
+  void printJson(raw_ostream &Out, const LocationContext *LCtx = nullptr,
+                 const char *NL = "\n", const char *Sep = "",
+                 unsigned int Space = 0, bool IsDot = false) const;
+
+  void printDOT(raw_ostream &Out, const LocationContext *LCtx = nullptr,
+                unsigned int Space = 0) const;
 
   void dump() const;
 
 private:
   friend void ProgramStateRetain(const ProgramState *state);
   friend void ProgramStateRelease(const ProgramState *state);
 
   /// \sa invalidateValues()

include/clang/StaticAnalyzer/Core/PathSensitive/Store.h

StoreRef enterStackFrame(Store store,
                            const StackFrameContext *CalleeCtx);
 
   /// Finds the transitive closure of symbols within the given region.
   ///
   /// Returns false if the visitor aborted the scan.
   virtual bool scanReachableSymbols(Store S, const MemRegion *R,
                                     ScanReachableSymbols &Visitor) = 0;
 
-  virtual void print(Store store, raw_ostream &Out, const char* nl) = 0;
+  virtual void printJson(raw_ostream &Out, Store S, const char *NL,
+                         unsigned int Space, bool IsDot) const = 0;
 
   class BindingsHandler {
   public:
     virtual ~BindingsHandler();
 
     /// \return whether the iteration should continue.
     virtual bool HandleBinding(StoreManager& SMgr, Store store,
                                const MemRegion *region, SVal val) = 0;

lib/StaticAnalyzer/Core/ProgramState.cpp

if (store)
     stateMgr->getStoreManager().decrementReferenceCount(store);
   store = newStoreStore;
 }
 
 //===----------------------------------------------------------------------===//
 //  State pretty-printing.
 //===----------------------------------------------------------------------===//
 
-void ProgramState::print(raw_ostream &Out,
+void ProgramState::printJson(raw_ostream &Out, const LocationContext *LCtx,
                              const char *NL, const char *Sep,
-                         const LocationContext *LC) const {
+                             unsigned int Space, bool IsDot) const {
   // Print the store.
   ProgramStateManager &Mgr = getStateManager();
   const ASTContext &Context = getStateManager().getContext();
-  Mgr.getStoreManager().print(getStore(), Out, NL);
+  Mgr.getStoreManager().printJson(Out, getStore(), NL, Space, IsDot);
 
   // Print out the environment.
-  Env.print(Out, NL, Sep, Context, LC);
+  Env.print(Out, NL, Sep, Context, LCtx);
 
   // Print out the constraints.
   Mgr.getConstraintManager().print(this, Out, NL, Sep);
 
   // Print out the tracked dynamic types.
   printDynamicTypeInfo(this, Out, NL, Sep);
 
   // Print checker-specific data.
-  Mgr.getOwningEngine().printState(Out, this, NL, Sep, LC);
+  Mgr.getOwningEngine().printState(Out, this, NL, Sep, LCtx);
 }
 
-void ProgramState::printDOT(raw_ostream &Out,
-                            const LocationContext *LC) const {
-  print(Out, "\\l", "\\|", LC);
+void ProgramState::printDOT(raw_ostream &Out, const LocationContext *LCtx,
+                            unsigned int Space) const {
+  printJson(Out, LCtx, "\\l", "\\|", Space, /*IsDot=*/true);
 }
 
 LLVM_DUMP_METHOD void ProgramState::dump() const {
-  print(llvm::errs());
+  printJson(llvm::errs());
 }
 
 AnalysisManager& ProgramState::getAnalysisManager() const {
   return stateMgr->getOwningEngine().getAnalysisManager();
 }
 
 //===----------------------------------------------------------------------===//
 // Generic Data Map.

lib/StaticAnalyzer/Core/RegionStore.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/Attr.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Basic/JsonSupport.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h"
 #include "llvm/ADT/ImmutableMap.h"
BindingKey BindingKey::Make(const MemRegion *R, Kind k) {
   const RegionOffset &RO = R->getAsOffset();
   if (RO.hasSymbolicOffset())
     return BindingKey(cast<SubRegion>(R), cast<SubRegion>(RO.getRegion()), k);
 
   return BindingKey(RO.getRegion(), RO.getOffset(), k);
 }
 
 namespace llvm {
-  static inline
-  raw_ostream &operator<<(raw_ostream &os, BindingKey K) {
-    os << '(' << K.getRegion();
-    if (!K.hasSymbolicOffset())
-      os << ',' << K.getOffset();
-    os << ',' << (K.isDirect() ? "direct" : "default")
-       << ')';
-    return os;
-  }
+static inline raw_ostream &operator<<(raw_ostream &Out, BindingKey K) {
+  Out << "\"kind\": \"" << (K.isDirect() ? "Direct" : "Default")
+      << "\", \"offset\": ";
+
+    Out << K.getOffset();
+  else
+    Out << "null";
+
+  return Out;
 
-} // end llvm namespace
+} // namespace llvm
 
-#ifndef NDEBUG
 LLVM_DUMP_METHOD void BindingKey::dump() const { llvm::errs() << *this; }
-#endif
 
 //===----------------------------------------------------------------------===//
 // Actual Store type.
 //===----------------------------------------------------------------------===//
 
 typedef llvm::ImmutableMap<BindingKey, SVal>    ClusterBindings;
 typedef llvm::ImmutableMapRef<BindingKey, SVal> ClusterBindingsRef;
 typedef std::pair<BindingKey, SVal> BindingPair;
public:
   ///  binding associated with a region and its subregions.
   Optional<SVal> getDefaultBinding(const MemRegion *R) const;
 
   /// Return the internal tree as a Store.
   Store asStore() const {
     return asImmutableMap().getRootWithoutRetain();
   }
 
-  void dump(raw_ostream &OS, const char *nl) const {
-   for (iterator I = begin(), E = end(); I != E; ++I) {
-     const ClusterBindings &Cluster = I.getData();
-     for (ClusterBindings::iterator CI = Cluster.begin(), CE = Cluster.end();
-          CI != CE; ++CI) {
-       OS << ' ' << CI.getKey() << " : " << CI.getData() << nl;
-     }
-     OS << nl;
+  void printJson(raw_ostream &Out, const char *NL = "\n",
+                 unsigned int Space = 0, bool IsDot = false) const {
+    for (iterator I = begin(); I != end(); ++I) {
+      Indent(Out, Space, IsDot)
+          << "{ \"cluster\": \"" << I.getKey() << "\", \"items\": [" << NL;
+
+      ++Space;
+      const ClusterBindings &CB = I.getData();
+      for (ClusterBindings::iterator CI = CB.begin(); CI != CB.end(); ++CI) {
+        Indent(Out, Space, IsDot) << "{ " << CI.getKey() << ", \"value\": \""
+                                  << CI.getData() << "\" }";
+        if (std::next(CI) != CB.end())
+          Out << ',';
+        Out << NL;
+      }
+
+      --Space;
+      Indent(Out, Space, IsDot) << "]}";
+      if (std::next(I) != end())
+        Out << ',';
+      Out << NL;
     }
   }
 
-  LLVM_DUMP_METHOD void dump() const { dump(llvm::errs(), "\n"); }
+  LLVM_DUMP_METHOD void dump() const { printJson(llvm::errs()); }
 };
 } // end anonymous namespace
 
 typedef const RegionBindingsRef& RegionBindingsConstRef;
 
 Optional<SVal> RegionBindingsRef::getDirectBinding(const MemRegion *R) const {
   return Optional<SVal>::create(lookup(R, BindingKey::Direct));
 }
public: // Part of public interface to class.
   //===------------------------------------------------------------------===//
 
   RegionBindingsRef getRegionBindings(Store store) const {
     return RegionBindingsRef(CBFactory,
                              static_cast<const RegionBindings::TreeTy*>(store),
                              RBFactory.getTreeFactory());
   }
 
-  void print(Store store, raw_ostream &Out, const char* nl) override;
+  void printJson(raw_ostream &Out, Store S, const char *NL = "\n",
+                 unsigned int Space = 0, bool IsDot = false) const override;
 
   void iterBindings(Store store, BindingsHandler& f) override {
     RegionBindingsRef B = getRegionBindings(store);
     for (RegionBindingsRef::iterator I = B.begin(), E = B.end(); I != E; ++I) {
       const ClusterBindings &Cluster = I.getData();
       for (ClusterBindings::iterator CI = Cluster.begin(), CE = Cluster.end();
            CI != CE; ++CI) {
         const BindingKey &K = CI.getKey();
StoreRef RegionStoreManager::removeDeadBindings(Store store,
 
   return StoreRef(B.asStore(), *this);
 }
 
 //===----------------------------------------------------------------------===//
 // Utility methods.
 //===----------------------------------------------------------------------===//
 
-void RegionStoreManager::print(Store store, raw_ostream &OS,
-                               const char* nl) {
-  RegionBindingsRef B = getRegionBindings(store);
-  OS << "Store (direct and default bindings), "
-     << B.asStore()
-     << " :" << nl;
-  B.dump(OS, nl);
+void RegionStoreManager::printJson(raw_ostream &Out, Store S, const char *NL,
+                                   unsigned int Space, bool IsDot) const {
+  RegionBindingsRef Bindings = getRegionBindings(S);
+
+  Indent(Out, Space, IsDot) << "\"store\": ";
+
+  if (Bindings.isEmpty()) {
+    Out << "null," << NL;
+    return;
+  }
+
+  Out << '[' << NL;
+  Bindings.printJson(Out, NL, ++Space, IsDot);
+  Indent(Out, --Space, IsDot) << "]," << NL;
 }

test/Analysis/expr-inspection.c

-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s 2>&1 | FileCheck %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=debug.ExprInspection \
+// RUN:  -verify %s 2>&1 | FileCheck %s
 
 // Self-tests for the debug.ExprInspection checker.
 
 void clang_analyzer_dump(int x);
 void clang_analyzer_printState();
 void clang_analyzer_numTimesReached();
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0<int x>}}
   clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0<int x>) + -1}}
   int y = 1;
-  clang_analyzer_printState();
-  for (; y < 3; ++y)
+  for (; y < 3; ++y) {
     clang_analyzer_numTimesReached(); // expected-warning{{2}}
+
+    if (y == 2) {
+      int z = x > 13;
+      if (!z)
+        clang_analyzer_printState();
+    }
+  }
 }
 
-// CHECK: Store (direct and default bindings)
-// CHECK-NEXT: (y,0,direct) : 1 S32b
+// CHECK:      "store": [
+// CHECK-NEXT:   { "cluster": "y", "items": [
+// CHECK-NEXT:     { "kind": "Direct", "offset": 0, "value": "2 S32b" }
+// CHECK-NEXT:   ]}
+// CHECK-NEXT: ]
 
-// CHECK: Expressions by stack frame:
+// CHECK:      Expressions by stack frame:
 // CHECK-NEXT: #0 Calling foo
-// CHECK-NEXT: clang_analyzer_printState : &code{clang_analyzer_printState}
+// CHECK-NEXT: (LC1, S847) clang_analyzer_printState : &code{clang_analyzer_printState}
 
-// CHECK: {{(Ranges are empty.)|(Constraints:[[:space:]]*$)}}
+// CHECK:      Ranges of symbol values:
+// CHECK-NEXT:  reg_$0<int x> : { [-2147483648, 13] }