[PATCH] D104550: [analyzer] Implement getType for SVal

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

I'm in favor of this patch. It will help simplify `SValBuilder::evalCast`, which takes an optional parameter `OriginalTy` and acts differently based on whether it has been passed or has not.

Since `sizeof(SVal)` became bigger (x1.5). I'm wondering of how much this reflects on memory consumption.

Another thing is that we can garantee returning `QualType`. I mean, we can replace `Optional` with `QualType` itself. `QualType` has a default ctor and `isNull` predicate, which is //true// when defaultly constructed.



================
Comment at: clang/lib/StaticAnalyzer/Core/SVals.cpp:154
+  Optional<QualType> VisitLocGotoLabel(loc::GotoLabel GL) {
+    return QualType{Context.VoidPtrTy};
+  }
----------------
I'm not sure this is a correct type. I would expect here something like: `class LabelType : public Type`.


================
Comment at: clang/unittests/StaticAnalyzer/SValTest.cpp:181-182
+void foo(int a, int b) {
+  int x = a;
+  int y = a + b;
+})") {
----------------
Add a cast case.


================
Comment at: clang/unittests/StaticAnalyzer/SValTest.cpp:190-192
+  SVal Y = getByName("y");
+  ASSERT_TRUE(Y.getType(Context).hasValue());
+  EXPECT_EQ(Int, *Y.getType(Context));
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104550