[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

I like this patch overall.. here are some stylistic nits.



================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:610
+            } else {
+              if (*lInt >= *rInt) {
+                newRhsExt = lInt->getExtValue() - rInt->getExtValue();
----------------
you can use `else if` here


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:595
+
+          if (origWidth < 128) {
+            auto newWidth = std::max(2 * origWidth, (uint32_t) 8);
----------------
I would like that "128" is rewritten somehow. Using expression instead.


================
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:596
+          if (origWidth < 128) {
+            auto newWidth = std::max(2 * origWidth, (uint32_t) 8);
+            auto newAPSIntType = APSIntType(newWidth, false);
----------------
Is `origWidth < 4` possible?

I wonder about "8". Is that CHAR_BIT hardcoded?


https://reviews.llvm.org/D35109