[PATCH] D45417: [analyzer] Don't crash on printing ConcreteInt of size >64 bits

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/SVals.cpp:304
       const nonloc::ConcreteInt& C = castAs<nonloc::ConcreteInt>();
-      if (C.getValue().isUnsigned())
-        os << C.getValue().getZExtValue();
-      else
-        os << C.getValue().getSExtValue();
-      os << ' ' << (C.getValue().isUnsigned() ? 'U' : 'S')
-         << C.getValue().getBitWidth() << 'b';
+      bool IsSigned = C.getValue().isSigned();
+      // FIXME: We can just call C.getValue().print() for all cases, but it has
----------------
Given that `getValue` is accessed 6 times now, I would put it into a variable


================
Comment at: lib/StaticAnalyzer/Core/SVals.cpp:305
+      bool IsSigned = C.getValue().isSigned();
+      // FIXME: We can just call C.getValue().print() for all cases, but it has
+      //        rather slow performance (see implementation of toString()).
----------------
Makes sense, another option would be to add a fast path to `APInt::print`?


================
Comment at: test/Analysis/egraph-dump-int128.c:3
+// RUN: mkdir -p %t.dir
+// RUN: env TMPDIR=%t.dir TEMP=%t.dir TMP=%t.dir %clang_analyze_cc1 -analyzer-checker=debug.ViewExplodedGraph %s
+
----------------
At least on a mac, `ViewExplodedGraph` launches `GraphViz.app`.
Can we have a less invasive check? Wouldn't just dumping a value be sufficient?


Repository:
  rC Clang

https://reviews.llvm.org/D45417