- User Since
- Feb 7 2019, 5:16 AM (31 w, 4 d)
Fri, Sep 6
Usually I'm not a big fan of auto, but in this case it makes sense not to duplicate the type name in the same expression. I'll use this rule of thumb in the future, thanks :)
Thank you for the suggestion. I didn't know about this behavior of %check_clang_tidy.
Aug 17 2019
The checker message has been changed according to the suggestion.
The last parameter of checkNonNull() doesn't require a default or optional value, so it has been fixed.
The parameter has been renamed from Idx to IdxOfArg.
Aug 16 2019
Thank you for the valuable comments and the review!
I'm just planning to register to the bug tracker, because it is getting more relevant due to some other contributions as well.
Aug 14 2019
Since now the order of diagnostic messages is deterministic, we can count on this order in the test file.
Aug 6 2019
This suggestion would result another strange behavior: if the user disables cert-err09-cpp because he or she doesn't want to see its reports, the other one (cert-err61-cpp) will still report the issue. So he or she has to disable both (or as many aliases it has).
Not exactly. The problem is that it is non-deterministic which checker reports the issue. For example before this patch, in the test file above there was only one report. However, sometimes the report message is:
Aug 5 2019
Alright, I modified the commit accordingly. Thank you for the suggestions.
Jul 23 2019
Yes, LessClangTidyError would really be the best place for this. The reason of my implementation was that I wanted to bind this feature to a command line option. I don't know if there was any design decision behind the current uniquing logic and I didn't want to break it if there was any. So would you be agree with having duplicate reports by default without any command line flag in case of alias checkers? I believe this would be the most painless solution, since in case of further improvements about including notes, fixes, etc. in the functor, the potential command line option controlling this would be unnecessarily complex. I don't know if there could be any use-case on those.
Thanks for the review!
Jul 22 2019
Mar 14 2019
I've uploaded another version of the last fix. The previous one contained an UB, although it worked practically.
Mar 13 2019
I added a condition before std::next() invocations to check if the next element is inside the valid interval. This fixes the crash of the build-bot. Sorry for the ugly bug.
I don't know if there is a more elegant solution.
Mar 12 2019
I rebased the patch on the current master.
Mar 7 2019
I added a test case for this recursive case. There is also a TODO in the code indicating the place where an additional fix will be required.
Feb 10 2019
Feb 8 2019
There was another place where this crash could have happened.
I've added a test case.