- User Since
- Nov 13 2019, 10:14 PM (54 w, 4 d)
May 4 2020
Apr 29 2020
Apr 14 2020
Removed redundant condition in check
Moved previously added condition into the actual matcher.
Apr 13 2020
Ran automatic formatting on the patch
Modified the check to not report static variables in function scope.
I'm also adding to this discussion the exception-part of the rule:
"A global object is often better than a singleton."
Apr 4 2020
After looking in to it I got less certain of where the error lies and eventually I got uncertain if this even is a false negative, so I started out with just posting a change where the unit test is updated to detect the issue.
This is just to have a start for a discussion of how this should actually work and what is the best way forward.
Apr 1 2020
After looking more closely at the code I think the issue is within hasLocalStorage() which is called in hasGlobalStorage(). My expectation would be that anything inside of function scope would be considered local but I'm not very certain.
Any thoughts on whether hasLocalStorage() should be modified or if I should change the check and use some more ad-hoc implementation, instead of hasGlobalStorage(), to determine if the variable is local or global?
Mar 31 2020
Mar 23 2020
Mar 13 2020
Mar 12 2020
Made example code in documentation legal C++
Mar 10 2020
Feb 24 2020
One can't really expect a ones own .clang-tidy file to be applicable to all third party code used in ones project. It is a fact, with or without this check, that if you use third party libraries you can't expect the third party libraries to comply with your own .clang-tidy file. You should expect to set up a separate .clang-tidy file for all third party libraries used by your project with or without this check -and that's ok because it is arguably not a big effort to do so.
I agree with this, the way I see it there are false positives and there is nothing "mushy" about the definitions of those.
Though I would prefer that, even if the implementation suggested in the guideline would be to crude, any deviations from what the C++ Core Guidelines specifies must be explicitly chosen by the user and not an option default.
Nice! Do you have any numbers on the amount of false positives this check produces, or the ratio of false and true positives?
I recently started work on this check myself but didn't get very far before I discovered this patch and abandoned mine.
Before abandoning I got some feedback though; there were concerns that there would be way to many false positives.
Will proceed with instead contributing however I can to the more mature patch https://reviews.llvm.org/D69560
Discovered duplication: https://reviews.llvm.org/D69560
- how do we either not warn on this by default or how does the user tell us to not warn on it (without requiring them to jump through hoops like changing the types of the arguments)?
-I'v used comments in the source code to tell the tool to ignore cases that I'v identified as false positives. That has worked without any issues for me and I wouldn't say it's a hassle. Is that no longer supported in clang tidy or was I using another tool and just projected that memory on clang-tidy?
I'm confident that clang-format atleast has a means of locally suppressing rules.
Feb 23 2020
I still feel a bit reluctant since the potential false positives would at least be left to the user to decide how to handle, while the false negatives from this option would just be lost without indication.
I don't see much value left in this check if one decides to use the option, anyone who would use this option might be better of just doing this check manually instead, then again it's optional of course.
If there wouldn't be much of a loss in performance I guess there isn't any significant harm in adding the option though, I'll give it a shot.
Feb 16 2020
Feb 11 2020
Updating D74463: Add new check avoid-adjacent-unrelated-parameters-of-the-same-type
The pull request to C++ Core guidelines have been merged: https://github.com/isocpp/CppCoreGuidelines/pull/1553
Updating D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check
Jan 6 2020
Dec 31 2019
Dec 30 2019
@whisperity I think you misunderstood my comment. I was not trying to give a more correct description of the current definition of style-level severity in CodeChecker. I was trying to propose new definitions of the different severity levels that this patch propose for clang-tidy.
So the claim that my suggestion is not "true" is invalid since that isn't a matter of true or false, but merely a matter of how one defines the style level.
Dec 29 2019
It's very hard to write these kinds of definitions without ambiguity and plenty of subjective interpretation creeping in. I'll try my best to provide constructive feedback but I'm admittedly struggling a bit with providing helpful counter proposals.
Ideally these levels would be based on a data driven approach, like when we say that something is error prone we should be able to support that with data.