[PATCH] D79232: [analyzer] Refactor range inference for symbolic expressions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a subscriber: steakhal.
NoQ added a comment.

Very nice, i like this architecture.

@baloghadamsoftware @steakhal @ASDenysPetrov will you be able to plug D49074 <https://reviews.llvm.org/D49074>/D50256 <https://reviews.llvm.org/D50256>/D77792 <https://reviews.llvm.org/D77792>/D77802 <https://reviews.llvm.org/D77802>/D78933 <https://reviews.llvm.org/D78933> into this so that to separate algebra from pattern-matching and ensure no performance regressions? Or is something still missing?



================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:271-281
+  RangeSet VisitSymIntExpr(const SymIntExpr *Sym) {
+    return VisitBinaryOperator(Sym);
+  }
+
+  RangeSet VisitIntSymExpr(const IntSymExpr *Sym) {
+    return VisitBinaryOperator(Sym);
+  }
----------------
Can we replace these three with a single `VisitBinarySymExpr()`? Or is there too much template duck typing involved?


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:340-345
+    // TODO #2: We didn't go into the nested expressions before, so it
+    // might cause us spending much more time doing the inference.
+    // This can be a problem for deeply nested expressions that are
+    // involved in conditions and get tested continuously.  We definitely
+    // need to address this issue and introduce some sort of caching
+    // in here.
----------------
I think this is a must-have, at least in some form. We've been exploding like this before on real-world code, well, probably not with bitwise ops but i'm still worried.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79232/new/

https://reviews.llvm.org/D79232