[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. Thanks for the report @bjope and thanks for the action @manas and @steakhal ! Sorry guys, I should have run our CI safety net jobs for this patch. I tend to do that with all my solver related patches, I really don't know why I thought this was different. Repository:

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. In D106102#3082399 , @manas wrote: > Based on the information from debugger, the logs > show RangeSets `[0, 255]` and > `[INT_MIN, 0]` from different types >

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. Based on the information from debugger, the logs show RangeSets `[0, 255]` and `[INT_MIN, 0]` from different types are causing the failure. I tried producing a compact test case

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. In D106102#3081178 , @bjope wrote: > I get failures after having merged this patch: > [...] > Here is the crash.c input F19785350: crash.c > > > I see lots of these. Probably worth a revert.

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

bjope added a comment. I actually see lots of these as well: clang: ../lib/Support/APInt.cpp:284: int llvm::APInt::compareSigned(const llvm::APInt &) const: Assertion `BitWidth == RHS.BitWidth && "Bit widths must be same for comparison"' failed. Although I haven't verified if those are relat

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

bjope added a comment. I get failures after having merged this patch: /llvm/build-all/bin/clang -Xanalyzer -analyzer-output=text -Xanalyzer -analyzer-checker='nullability,optin.portability.UnixAPI,security,valist' -Xanalyzer -analyzer-disable-checker='security.insecureAPI.strcpy' --analyze c

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. In D106102#3080339 , @manas wrote: > I have received a failed build on clang-ppc64be-linux > (https://lab.llvm.org/buildbot/#/builders/52/builds/11704). Although, I > haven't verified the commit. Is there something wrong with th

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. I have received a failed build on clang-ppc64be-linux (https://lab.llvm.org/buildbot/#/builders/52/builds/11704). Although, I haven't verified the commit. Is there something wrong with this commit? Is the function VisitBinaryOperator reachable? Repository: rG LLVM Git

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. I've fixed the compilation error and pushed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

This revision was automatically updated to reflect the committed changes. Closed by commit rGcac8808f154c: [analyzer][solver] Introduce reasoning for not equal to operator (authored by manas, committed by martong). Changed prior to commit: https://reviews.llvm.org/D106102?vs=376363&id=381496#to

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. I'll take a look at this over the weekend. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. It seems like it doesn't build with GCC 8.3.0 on a Debian system. Could you investigate? llvm-project/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1029:13: error: explicit specialization in non-namespace scope 'class {anonymous}::SymbolicRangeInferrer'

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. In D106102#3036474 , @steakhal wrote: > In D106102#3036220 , @manas wrote: > >> I do not have landing rights. > > Please add your name and email on whom behalf I should commit this patch. >

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. 🛬 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. In D106102#3036220 , @manas wrote: > I do not have landing rights. Please add your name and email on whom behalf I should commit this patch. Mine is `Balazs Benics` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. In D106102#3035598 , @steakhal wrote: > Good work. Land it. I do not have landing rights. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal accepted this revision. steakhal added a comment. Good work. Land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas updated this revision to Diff 376363. manas added a comment. Fix typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp clang/test/A

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. In D106102#3032888 , @steakhal wrote: > Okay, I don't see any problems besides this typo. Oops! > BTW do you plan to implement other rules besides this in the future? > E.g. we currently miss this: > 'x < y' is `true` if `max(x) <

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. Okay, I don't see any problems besides this typo. BTW do you plan to implement other rules besides this in the future? E.g. we currently miss this: 'x < y' is `true` if `max(x) < min(y)`; and `false` if `min(x) >= max(y)` 'x > y', 'x <= y', etc. in a similar way.

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas updated this revision to Diff 376035. manas added a comment. Apply mentioned edits in testcases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 Files: clang/lib/StaticAnalyzer/Core/RangeConstraint

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. In D106102#3029633 , @martong wrote: > In D106102#3028584 , @manas wrote: > >> The pre-merge checks fail due to the patch being unable to get applied. The >> troubleshooting >>

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

steakhal added a comment. This patch seems pretty straightforward, good job @manas, and for the folks giving review comments. Aside from polishing the tests, I think it's good to go. Comment at: clang/test/Analysis/constant-folding.c:470 + +void testEqualityRules(unsigned int

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. In D106102#3028584 , @manas wrote: > The pre-merge checks fail due to the patch being unable to get applied. The > troubleshooting > > suggest

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. The pre-merge checks fail due to the patch being unable to get applied. The troubleshooting suggest to update the patch via `arc diff ```git merge-base HEAD origin``` --update D106102` an

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas updated this revision to Diff 375689. manas added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp clang/test/Ana

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas updated this revision to Diff 375682. manas added a comment. Move method to a specialized template for VisitBinaryOperator Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 Files: clang/lib/StaticAn

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a reviewer: steakhal. martong added a comment. Adding @steakhal as reviewer because of other reviewers inactivity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. In D106102#3019921 , @manas wrote: > I haven't tried specializing that `VisitBinaryOperator` method which converts > Ranges from RangeSets (as @vsavchenko mentioned). Should this case for NE > stay here in the switch or move? P

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas updated this revision to Diff 374746. manas added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 Files: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp clang/test/Ana

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added a comment. I haven't tried specializing that `VisitBinaryOperator` method which converts Ranges from RangeSets (as @vsavchenko mentioned). Should this case for NE stay here in the switch or move? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

martong added a comment. This is promising! Gentle ping @manas @vsavchenko Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106102/new/ https://reviews.llvm.org/D106102 ___ cfe-commits mailing list cfe-comm

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1223-1225 +template <> +RangeSet SymbolicRangeInferrer::VisitBinaryOperator(Range LHS, Range RHS, + QualType T) { -

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

vsavchenko added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1223-1225 +template <> +RangeSet SymbolicRangeInferrer::VisitBinaryOperator(Range LHS, Range RHS, + QualType T) {

[PATCH] D106102: [analyzer][solver] Introduce reasoning for not equal to operator

manas created this revision. Herald added subscribers: steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: teemperor. manas requested review of this revision. Herald added a project: clang. Her