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

Denys Petrov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 24 15:14:21 PDT 2021


ASDenysPetrov added a comment.

> I'm not sure what you mean here. If it is about this particular patch, it simply adds a non-virtual method to SVal, it shouldn't affect sizeof(SVal) at all.

My fault. For some reason I thought you added a QualType as a member to SVal declaration.

> I wanted to be more explicit here and convey to the user that the lack of type is normal.

We need to think about what real usage would look and feel better.

My another proposition is to think about how we can rework `ConcreteInt` to `Int` which can keep ranges. Previosly we couldn't keep range sets inside Sval as they were ImmutableSets, but now they are just pointers to vectors. So we can pass the range set inside SVal. IMO now RangeConstraintManager is smart enough to do standart arithmetics with ranges as we do for concrete ints. These are just my recent thoughts out loud.



================
Comment at: clang/lib/StaticAnalyzer/Core/SVals.cpp:154
+  Optional<QualType> VisitLocGotoLabel(loc::GotoLabel GL) {
+    return QualType{Context.VoidPtrTy};
+  }
----------------
vsavchenko wrote:
> ASDenysPetrov wrote:
> > I'm not sure this is a correct type. I would expect here something like: `class LabelType : public Type`.
> I don't think that I fully understood what you suggest here.  Do you suggest to add a new type to `Type.h`?
Yes. As a user I expect to see some special type for labels, not a `void*`. But for the absence of such type let it be as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104550



More information about the cfe-commits mailing list