vsavchenko added a comment.
In D104550#2838827 <https://reviews.llvm.org/D104550#2838827>, @ASDenysPetrov
wrote:
> 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.
@NoQ mentioned it before that it is always better to use direct types from AST.
But the only participant who doesn't have AST is the Store, and when it calls
`evalCast` it doesn't have a way to get that type. So, in this case this
function van be pretty useful.
> Since `sizeof(SVal)` became bigger (x1.5). I'm wondering of how much this
> reflects on memory consumption.
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.
> 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.
I wanted to be more explicit here and convey to the user that the lack of type
is normal. I can change that, it won't be a problem at all. @NoQ, @Szelethus
what do you think?
================
Comment at: clang/lib/StaticAnalyzer/Core/SVals.cpp:154
+ Optional<QualType> VisitLocGotoLabel(loc::GotoLabel GL) {
+ return QualType{Context.VoidPtrTy};
+ }
----------------
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`?
================
Comment at: clang/unittests/StaticAnalyzer/SValTest.cpp:181-182
+void foo(int a, int b) {
+ int x = a;
+ int y = a + b;
+})") {
----------------
ASDenysPetrov wrote:
> Add a cast case.
Sure!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104550/new/
https://reviews.llvm.org/D104550
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
