- User Since
- Mar 9 2016, 4:07 AM (162 w, 2 d)
Tue, Apr 16
Thu, Mar 28
Fixed double transition.
Mar 19 2019
This one seems straightforward.
Great work! Thank you! I only have minor comment: did you consider moving the refactoring of ExceptionAnalyzer into a separate (prerequisite) patch?
Mar 18 2019
No patch should be committed without clang-formatting anyway...
Please rename the patch. Its name does not really express its content.
Aside from little comment it seems OK.
Ping! Any news regarding this patch?
Mar 13 2019
Mar 12 2019
This is straightforward.
Mar 11 2019
Feb 21 2019
Feb 20 2019
Instead of recording comparisons do an eager state split if the result is a symbolic value.
Great! However, I would rename the test files: modernize-use-noexcept-opt.cpp is inaccurate since there are multiple options, it could be e.g. modernize-use-noexcept-dont-use-noexcept-false.cpp. modernize-use-noexcept-new-noexcept.cpp is a bit misleading because of the new which is a valid C++ keyword. modernize-use-noexcept-add-missing-noexcept.cpp sounds better for me.
Thank you for working this. I totally agree with you: whenever the checker spawns a new node in the exploded graph there is no point to leave it unmarked and then revers engineer it. BugReporterVisitors should only care for nodes which are not spawned by the checker reporting the bug. This way we could get rid of some unnecessary visitors, e.g. CXXSelfAssignmentBRVisitor. This also opens an easier way for checkers similar to CXXSelfAssignmentChecker which does not report any bug but creates a new execution path which may end in a bug reported by another checker. Using note tags there is no need to create a separate BugReporterVisitor for every such checker and add it globally to every bug report.
Feb 19 2019
If I understand it correctly, this is more of an infrastructure improvement than check enhancement. It looks like a nice and clean code. Where do we expect to use this new behavior? In the current check or in the upcoming "modernize" check?
Feb 18 2019
Feb 15 2019
Feb 13 2019
Feb 12 2019
Function renamed, comment updated.
Feb 11 2019
I tried very hard to create a test case where we are crashing on a true positive but I did not succeed. I am not sure whether it is possible so fixing the false positive in CallAndMessageUnInitRefArg also fixes the crash without this patch. However I am confident the bug is still a bug in the visitor and maybe in the future it will be used for complex values as well which could be LazyCompoundVals. Also you can see in the test case uninit-vals.m that even if it does not crash it prints nonsense bug path notes caused by this same bug which is fixed by this patch.
Jan 30 2019
Jan 28 2019
Hello! I am glad to see that this check gets improved by the community. I also think a "modernize" check which marks functions with noexcept is also useful.
Jan 21 2019
Jan 16 2019
I think it is straightforward. Thank you for noticing this.
Jan 14 2019
This seems to be an important fix. Thank you!
Dec 5 2018
Dec 4 2018
Thank you for reviewing this patch!
My original idea was that once we ony store either A - B or B - A. Thus if we already have A - B stored then do not store range for B - A but negate both the difference and the range. I can think on two ways to implement this:
Dec 3 2018
One error message slightly updated.
Now I think I understand the terminology and the concept so I could add Doxygen comment. I also refactored the code as you suggested, original code was based on getBaseRegion().
Dec 1 2018
Thank you for the review!
Nov 30 2018
Committed because I was notified that the deadline for 7.0.1 is today.
Nov 29 2018
Nov 28 2018
Nov 26 2018
Nov 23 2018
Updated according to the comments.
More standard-like tests.
Restored corrected patch after uploading an incorrect one.
There is CXXDerivedObjectRegion as well. I am totally confused about the terminology now. Is there somewhere a documentation that explains all these things? If I make a class hierarchy, then the region of derived objects are CXXBaseObjectRegion. I did not meet CXXDerivedObjectRegion yet.
Nov 21 2018
Nov 15 2018
Here is the proof of the algorithm for signed char and unsigned char types. The dumper which tests every n for the n*k <comparison operator> m relation and creates ranges from values satisfying the expression for every k and m produces exactly the same output as the generator which generates the ranges using the algorithm from the patch. One the machine I run them they take between 30 and 60 seconds to run.
Rebased on the previous part. Tests updated for the now default eagerly assume mode.
Tests updated to the now default eagerly assume mode. Range scaling fixed.
Nov 13 2018
I marked this patch as WIP because I could not create a test-case for it. However in real projects this patch seems to reduce false positives significantly.
Nov 6 2018
Nov 5 2018
Your suggestion sounds completely reasonable. We cannot rely entirely on inlining of comparison operators. However, there is an important side-effect of inlining: on iterator-adapters inlining ensures that we handle the comparison of the underlying iterator correctly. Without inlining, evalCall() only works on the outermost iterator which is not always correct: it may happen that the checker cannot conclude any useful relation between the two iterator-adapters but there are some relations stored about the inner iterators. Based on my experiments quite many of the false-positives are related to iterator-adapters. So I am afraid that we introduce even more false-positives by losing inlining.
Oct 30 2018
@NoQ Any comments/concerns regarding this solution?