[PATCH] D139113: [clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace

vingeldal accepted this revision. vingeldal added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp:46-48 +// OK +static const int v8{123}; +static constexpr int v9{123};

[PATCH] D79113: Revert "Remove false positive in AvoidNonConstGlobalVariables."

vingeldal added a comment. In D79113#2016294 , @aaron.ballman wrote: > I only see a comment on the issue, but not that the issue was resolved in > favor of that comment (unless I've missed something). If the issue gets > resolved in the direction that c

[PATCH] D79113: Revert "Remove false positive in AvoidNonConstGlobalVariables."

vingeldal created this revision. Herald added subscribers: cfe-commits, kbarton, nemanjai. Herald added a project: clang. vingeldal added reviewers: aaron.ballman, lebedev.ri, JonasToth, gribozavr2. Herald added a subscriber: wuzish. There was concernes about a false positive in clang-tidy check A

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal updated this revision to Diff 257423. vingeldal marked an inline comment as done. vingeldal added a comment. Removed redundant condition in check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77461/new/ https://reviews.llvm.org/D77461 Fi

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:50 +// variables in a function. +if (!Variable->isLocalVarDecl()) { + diag(Variable->getLocation(), "variable %0 is non-const and global

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal updated this revision to Diff 257335. vingeldal marked 3 inline comments as done. vingeldal added a comment. Moved previously added condition into the actual matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77461/new/ https://review

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp:234-236 +static int staticNonConstLoopVariable = 42; int nonConstLoopVariable = 42; +nonConstInt = nonConstLoopVariable + i

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal updated this revision to Diff 257060. vingeldal added a comment. Ran automatic formatting on the patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77461/new/ https://reviews.llvm.org/D77461 Files: clang-tools-extra/clang-tidy/cppcor

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal updated this revision to Diff 257058. vingeldal added a comment. Modified the check to not report static variables in function scope. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77461/new/ https://reviews.llvm.org/D77461 Files: clan

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added a comment. In D77461#1977703 , @aaron.ballman wrote: > In D77461#1977687 , @vingeldal wrote: > > > In D77461#1977639 , @aaron.ballman > > wrote: > > > > > I

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added a comment. I'm also adding to this discussion the exception-part of the rule: "A global object is often better than a singleton." I would argue that the example given above, while not strictly follwong the definition of a singleton, is behaviorally consistent with a singleton.

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added a comment. In D77461#1977639 , @aaron.ballman wrote: > In D77461#1977503 , @vingeldal wrote: > > > In D77461#1963166 , @lebedev.ri > > wrote: > > > > > http

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added a comment. In D77461#1963166 , @lebedev.ri wrote: > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global > is in "Interfaces" section, it only covers inter-procedural stuff. > Diagnosing function-local st

[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal created this revision. Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai. Herald added a project: clang. vingeldal added a comment. Herald added a subscriber: wuzish. After looking in to it I got less certain of where the error lies and eventually I got uncertain if th

[PATCH] D77461: [clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

vingeldal added a comment. Herald added a subscriber: wuzish. 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

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal added a comment. 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 hasLo

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal added a comment. In D70265#1953335 , @lebedev.ri wrote: > Hello. > 2 points: > > rG512767eb3fe9c34c655a480d034147c54f1d4f85 > doesn't > reference this review, > so it's a b

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

vingeldal added a comment. In D69560#1935071 , @whisperity wrote: > @aaron.ballman I've gone over LLVM (and a few other projects). Some general > observations: > > - Length of `2` **is vile**. I understand that the C++CG rule says even > lengths of 2 sho

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal added a comment. In D70265#1920379 , @aaron.ballman wrote: > LGTM! Do you need someone to commit on your behalf? Yes please. Do I need to, or can I, do anything about the failing builds (https://reviews.llvm.org/B49012) first? They seem unrel

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:55-71 + if (const auto *Reference = + Result.Nodes.getNodeAs("reference_to_non-const")) { +diag(Reference->getLocation(), + "va

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal updated this revision to Diff 249980. vingeldal marked 6 inline comments as done. vingeldal added a comment. Made example code in documentation legal C++ Changed implementation according to comments so that both the pointer matcher and the reference matcher now bind to the same id Rep

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:55-71 + if (const auto *Reference = + Result.Nodes.getNodeAs("reference_to_non-const")) { +

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

vingeldal added a comment. In D69560#1890026 , @aaron.ballman wrote: > In D69560#1889925 , @vingeldal wrote: > > > Doesn't clang-tidy exclude warnings from system headers by default though? > > > Third-party SDKs a

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

vingeldal added a comment. In D69560#1889719 , @aaron.ballman wrote: > In D69560#1889483 , @whisperity > wrote: > > > I'd rather not call them //false positive// because the definition of > > `false positive` is

[PATCH] D69560: [clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

vingeldal added a comment. 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

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal abandoned this revision. vingeldal added a comment. Will proceed with instead contributing however I can to the more mature patch https://reviews.llvm.org/D69560 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74463/new/ https://reviews.ll

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal added a comment. Discovered duplication: https://reviews.llvm.org/D69560 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74463/new/ https://reviews.llvm.org/D74463 ___ cfe-commits mailing list

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal added a comment. In D74463#1889157 , @vingeldal wrote: > > - 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 argume

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal added a comment. > - 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

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal added a comment. In D74463#1878378 , @njames93 wrote: > In D74463#1878290 , @vingeldal wrote: > > > I am pretty sure that an option to allow short names would cause a > > relatively big hit on performanc

[PATCH] D74463: [clang-tidy] Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal marked 2 inline comments as done. vingeldal added a comment. In D74463#1878144 , @njames93 wrote: > I have a feeling this check is going to throw so many false positives that > it'll be too noisy to run on any real codebase. > There should be

[PATCH] D74463: Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal updated this revision to Diff 244072. vingeldal added a comment. Updating D74463 : Add new check avoid-adjacent-unrelated-parameters-of-the-same-type - Changed commit message to start with [clang-tidy] Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D74463: Add new check avoid-adjacent-unrelated-parameters-of-the-same-type

vingeldal created this revision. Herald added subscribers: cfe-commits, kbarton, mgorny, nemanjai. Herald added a project: clang. This check is part of the C++ Core Guidelines, rule I.24 https://github.com/vingeldal/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-unrelated Repository: rG

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal added a comment. The pull request to C++ Core guidelines have been merged: https://github.com/isocpp/CppCoreGuidelines/pull/1553 Herb set me straight regarding the anonymous namespaces so I had to change that. While waiting for the pull request to be reviewed I looked at more guideline

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

vingeldal updated this revision to Diff 243757. vingeldal added a comment. Updating D70265 : [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check Created the requested pull request to C++ Core guidelines and since it is now merged this

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:198 cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) + cppcoreguidelines-avoid-non-const-global-variabl

[PATCH] D71963: clang-tidy doc: Add the severities description

vingeldal added a comment. In D71963#1799956 , @aaron.ballman wrote: > I may have missed this in prior discussions, and if so, I'm sorry -- but why > are we taking CodeChecker as the model for this? I'm not opposed to having > some notion of severity fo

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:110 + "making it const") + << MemberVariable; // FIXME: Add fix-it hint to MemberVaria

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal updated this revision to Diff 235600. vingeldal marked 2 inline comments as done. vingeldal added a comment. Updating D70265 : [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines - Fixed code example in documentation to make it valid code. - Fixe

[PATCH] D71963: clang-tidy doc: Add the severities description

vingeldal added a comment. @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

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:29 + unless(anyOf( + isConstexpr(), hasDeclContext(namespaceDecl(isAnonymous())), +

[PATCH] D71963: clang-tidy doc: Add the severities description

vingeldal added a comment. 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 leve

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked 2 inline comments as done. vingeldal added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:198 cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) + cppcoreguidelines-avoid-non-const-global-variabl

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal updated this revision to Diff 235483. vingeldal added a comment. Updating D70265 : [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines - Adjusted the line breaks of some string literals. - Modified list.rst Repository: rG LLVM Github Monorepo

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:29 + unless(anyOf( + isConstexpr(), hasDeclContext(namespaceDecl(isAnonymous())), +

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal marked an inline comment as done. vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:123-124 + "member variable %0 provides global access to non-const type, " + "consider

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:64 + + if (Variable) { +diag(Variable->getLocation(), "variable %0 is non-const and globally " aaron.ballman wrote: > vingeldal

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal updated this revision to Diff 235474. vingeldal marked 12 inline comments as done. vingeldal added a comment. Updating D70265 : [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines - Updated list.rst - changed: "pointed to" -> "pointed-to" - CHang

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal updated this revision to Diff 235176. vingeldal marked 7 inline comments as done. vingeldal added a comment. Updating D70265 : [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines - Added an option to not check member variables. - Add checking for

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:64 + + if (Variable) { +diag(Variable->getLocation(), "variable %0 is non-const and globally " JonasToth wrote: > each of those

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal updated this revision to Diff 234863. vingeldal marked 5 inline comments as done. vingeldal added a comment. Updating D70265 : [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines Adressed all comments: - Extended teseting - Updated documentation

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:20 +namespace { +AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); } +} // namespace JonasToth wrote: > This matc

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

vingeldal created this revision. vingeldal added reviewers: JonasToth, aaron.ballman. Herald added subscribers: kbarton, xazax.hun, mgorny, nemanjai. Herald added a project: clang. Cpp Core Guideline I.2, a.k.a "Avoid non-const global variables" For detailed documentation, see: https://github.com/