[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3181580 , @NoQ wrote: > Like, that's the whole reason why `nonloc::LocAsInteger` exists: so that we > could cast a pointer to an integer and actually have a way to represent the > resulting value as `NonLoc`. > I'm

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3195377 , @ASDenysPetrov wrote: > @martong > Thanks for clarification. > >> TLDR, it is recommended to use the arcanist. > > I'm not able to use arcanist. It doesn't work on Windows (at least I've tryed > several

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added a comment. @martong Thanks for clarification. > TLDR, it is recommended to use the arcanist. I'm not able to use arcanist. It doesn't work on Windows (at least I've tryed several ways to set up it). BTW. I found a revision from which the

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3192725 , @ASDenysPetrov wrote: > @martong wrote: > >> Denis, you can see in the `Revision Contents` that Diff 3 has the baseline >> commit `63a6348`. When I check out `63a6348` then the newly added test file >>

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added a comment. @martong wrote: > Denis, you can see in the `Revision Contents` that Diff 3 has the baseline > commit `63a6348`. When I check out `63a6348` then the newly added test file > triggers the assertion about `BO_Add`. Yes is see it: F21029827: image.png

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3188743 , @ASDenysPetrov wrote: >> @steakhal >> I don't get this one. I've provided a bunch of tests, even annotated with >> `no-crash` comments where we crashed prior to this change. > > I wasn't able to catch any

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added a comment. > @steakhal > I don't get this one. I've provided a bunch of tests, even annotated with > `no-crash` comments where we crashed prior to this change. I wasn't able to catch any crashes with your tests file //(symbol-simplification-nonloc-loc.cpp)// on the baseline

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

steakhal marked an inline comment as done. steakhal added a comment. In D115149#3182196 , @ASDenysPetrov wrote: > @steakhal > Please provide a case which asserts before your patch. I don't get this one. I've provided a bunch of tests, even annotated

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:463 + if (const Optional RV = rhs.getAs()) { +const auto IsCommutative = [](BinaryOperatorKind Op) { + return Op == BO_Mul || Op == BO_Add || Op == BO_And || Op == BO_Xor ||

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added a comment. @steakhal @NoQ wrote: > I'm confident that there's a way to get it right, simply because the program > under analysis is type-correct. If it's the simplifier, let's fix the > simplifier. I agree with your main thought. But I also believe there definitely are

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

NoQ added a comment. Like, that's the whole reason why `nonloc::LocAsInteger` exists: so that we could cast a pointer to an integer and actually have a way to represent the resulting value as `NonLoc`. I'm confident that there's a way to get it right, simply because the program under analysis

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

NoQ added a comment. > There is the reinterpret-cast operation which is capable of crossing these > two domains, producing an expression that can participate in arithmetic > operations, but on the abstract domain side, we still stick to Locs Such cast should turn the `loc::ConcreteInt` into a

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added a comment. It seems like your test file passes even before your patch. I've just checked it. My last pull from the baseline was on Nov 15. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115149/new/

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

steakhal marked an inline comment as done. steakhal added a comment. Sorry for my late response, I have a bunch of other tasks to do. --- In D115149#3175068 , @NoQ wrote: >> It can happen if the `Loc` was perfectly constrained to a concrete >> value

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

ASDenysPetrov added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:463 + if (const Optional RV = rhs.getAs()) { +const auto IsCommutative = [](BinaryOperatorKind Op) { + return Op == BO_Mul || Op == BO_Add || Op == BO_And || Op == BO_Xor ||

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

martong added a comment. In D115149#3175068 , @NoQ wrote: >> It can happen if the `Loc` was perfectly constrained to a concrete >> value (`nonloc::ConcreteInt`) > > This shouldn't happen. It should be `loc::ConcreteInt` which is, well, a > `Loc`. I see

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

NoQ added a comment. > It can happen if the `Loc` was perfectly constrained to a concrete > value (`nonloc::ConcreteInt`) This shouldn't happen. It should be `loc::ConcreteInt` which is, well, a `Loc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

This revision was automatically updated to reflect the committed changes. Closed by commit rGa6816b957d28: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions (authored by steakhal). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github