github-actions[bot] wrote:
@pascalj Congratulations on having your first Pull Request (PR) merged into the
LLVM Project!
Your changes will be combined with recent changes from other authors, then
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with
a
https://github.com/PiotrZSL closed
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL updated
https://github.com/llvm/llvm-project/pull/93827
>From abe862b84fd6915edb281a21e49f1be2aac3a626 Mon Sep 17 00:00:00 2001
From: Pascal Jungblut
Date: Thu, 30 May 2024 14:28:50 +0200
Subject: [PATCH] Add option to ignore anonymous namespaces in
https://github.com/HerrCai0907 approved this pull request.
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/5chmidti approved this pull request.
While I'm leaning more towards
> NOLINT in the code can grab the attention of a future developer and prompt a
> refactoring to avoid the pattern.
, it's an off-by-default option that provides a choice.
LGTM
5chmidti wrote:
FYI: You could remove the explicit check-nots here:
```c++
// RUN: %check_clang_tidy %s -check-suffixes=,INTERNAL-LINKAGE
cppcoreguidelines-avoid-non-const-global-variables %t
// RUN: %check_clang_tidy %s
https://github.com/5chmidti edited
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -7,21 +7,30 @@
//===--===//
#include "AvoidNonConstGlobalVariablesCheck.h"
-#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
https://github.com/SimplyDanny approved this pull request.
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/SimplyDanny edited
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL approved this pull request.
LGTM
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
pascalj wrote:
Thanks for your feedback!
> * what about "static" non const global variables
Good point, forgot about these. If both are allowed, it is more about the
internal linkage than it is about the namespace. I renamed the option to
`AllowInternalLinkage` and permit variables in
@@ -1,29 +1,39 @@
-// RUN: %check_clang_tidy %s
cppcoreguidelines-avoid-non-const-global-variables %t
+// RUN: %check_clang_tidy %s -check-suffix=DEFAULT
cppcoreguidelines-avoid-non-const-global-variables %t
+// RUN: %check_clang_tidy %s -check-suffix=NAMESPACE
https://github.com/pascalj updated
https://github.com/llvm/llvm-project/pull/93827
>From abe862b84fd6915edb281a21e49f1be2aac3a626 Mon Sep 17 00:00:00 2001
From: Pascal Jungblut
Date: Thu, 30 May 2024 14:28:50 +0200
Subject: [PATCH] Add option to ignore anonymous namespaces in
carlosgalvezp wrote:
I have a bit the feeling that users should use NOLINT* here instead of having a
global option that is not seen in the code. Anonymous namespaces do not solve
many of the remaining issues. A NOLINT in the code can grab the attention of a
future developer and prompt a
@@ -41,3 +41,25 @@ The variables ``a``, ``c``, ``c_ptr1``, ``c_const_ptr`` and
``c_reference``
will all generate warnings since they are either a non-const globally
accessible
variable, a pointer or a reference providing global access to non-const data
or both.
+
+Options
@@ -41,3 +41,25 @@ The variables ``a``, ``c``, ``c_ptr1``, ``c_const_ptr`` and
``c_reference``
will all generate warnings since they are either a non-const globally
accessible
variable, a pointer or a reference providing global access to non-const data
or both.
+
+Options
@@ -1,29 +1,39 @@
-// RUN: %check_clang_tidy %s
cppcoreguidelines-avoid-non-const-global-variables %t
+// RUN: %check_clang_tidy %s -check-suffix=DEFAULT
cppcoreguidelines-avoid-non-const-global-variables %t
+// RUN: %check_clang_tidy %s -check-suffix=NAMESPACE
@@ -16,9 +15,12 @@ using namespace clang::ast_matchers;
namespace clang::tidy::cppcoreguidelines {
void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
+ auto NamespaceMatcher = Options.get("AllowAnonymousNamespace", false)
https://github.com/PiotrZSL edited
https://github.com/llvm/llvm-project/pull/93827
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/PiotrZSL requested changes to this pull request.
- storeOptions is missing
- release notes need to be updated about added new option
- tests need some love (reduce amount of changes)
- what about "static" non const global variables
I'm fine for having an options that control
@@ -41,3 +41,25 @@ The variables ``a``, ``c``, ``c_ptr1``, ``c_const_ptr`` and
``c_reference``
will all generate warnings since they are either a non-const globally
accessible
variable, a pointer or a reference providing global access to non-const data
or both.
+
+Options
llvmbot wrote:
@llvm/pr-subscribers-clang-tidy
Author: Pascal Jungblut (pascalj)
Changes
Add an option to ignore warnings for cppcoreguidelines
avoid-non-const-global-variables.
Understandably, the core guidelines discourage non const global variables, even
at the TU level (see
github-actions[bot] wrote:
Thank you for submitting a Pull Request (PR) to the LLVM Project!
This PR will be automatically labeled and the relevant teams will be
notified.
If you wish to, you can add reviewers by using the "Reviewers" section on this
page.
If this is not working for you,
https://github.com/pascalj created
https://github.com/llvm/llvm-project/pull/93827
Add an option to ignore warnings for cppcoreguidelines
avoid-non-const-global-variables.
Understandably, the core guidelines discourage non const global variables, even
at the TU level (see
25 matches
Mail list logo
