[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think the overall idea is correct.

Yup, the patch will need the new positive tests that started passing. If you can't demonstrate it with integration tests you should do unit tests because the contract you're trying to achieve is fairly clear and obviously correct and it makes sense to test it separately.



================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1056
       }
+    } else if (LeftER && isa<SymbolicRegion>(RightMR)) {
+      if (LeftER->getSuperRegion() != RightMR)
----------------
Symbolic regions aren't special. Any other parent region should work similarly.


================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1059-1060
+        return UnknownVal();
+      if (Optional<NonLoc> LeftIndex = MaybeNonLocIndexOfElementRegion(LeftER))
+        return LeftIndex.getValue();
+      return UnknownVal();
----------------
You'll have to multiply by the size of a single element - it's an index, not an offset.


================
Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:1067-1069
+        llvm_unreachable(
+            "TODO: return the negated value of RightIndex - figure out how :D\n"
+            "Maybe a unary minus operator would do the job.");
----------------
Yeah, sounds like we'll have to stick to `UnknownVal` for now when `RightIndex` is symbolic. You can easily compute unary minus for a concrete index though, and that'll probably be your most important case anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84736