[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel updated this revision to Diff 203154.
koldaniel added a comment.
Updating warnings.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantExternCheck.cpp
clang-tidy/readability/RedundantExternCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/readability-redundant-extern.rst
test/clang-tidy/readability-redundant-extern.cpp
Index: test/clang-tidy/readability-redundant-extern.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-extern.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f();
+
+int extern f() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f() { return 0; }
+
+extern "C" int g();
+// CHECK-FIXES: extern "C" int g();
+
+extern "C" int g() { return 0; }
+// CHECK-FIXES: extern "C" int g() { return 0; }
+
+extern "C++" int h();
+// CHECK-FIXES: extern "C++" int h();
+
+extern "C++" int h() { return 0; }
+// CHECK-FIXES: extern "C++" int h() { return 0; }
+
+inline extern void foo_inline();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: inline void foo_inline();
+
+#define FOO_EXTERN extern
+FOO_EXTERN void foo_macro_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_EXTERN void foo_macro_1();
+
+#define FOO_INLINE inline
+FOO_INLINE extern void foo_macro_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_INLINE extern void foo_macro_2();
+
+#define FOO_EXTERN_INLINE inline extern
+FOO_EXTERN_INLINE void foo_macro_3();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_EXTERN_INLINE void foo_macro_3();
+
+void file_scope();
+// CHECK-FIXES: void file_scope();
+
+void another_file_scope(int _extern);
+// CHECK-FIXES: void another_file_scope(int _extern);
+
+namespace {
+extern void namespace_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'extern' keyword has no effect [readability-redundant-extern]
+// CHECK-FIXES: void namespace_1();
+}
+
+namespace a {
+namespace {
+namespace b {
+extern void namespace_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: 'extern' keyword has no effect [readability-redundant-extern]
+// CHECK-FIXES: void namespace_2();
+}
+}
+}
+
+namespace {
+extern "C" void namespace_3();
+// CHECK-FIXES: extern "C" void namespace_3();
+}
+
+#define FOO_EXTERN extern
+typedef int extern_int;
+
+extern_int FOO_EXTERN typedef_extern_foo();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: extern_int FOO_EXTERN typedef_extern_foo();
Index: docs/clang-tidy/checks/readability-redundant-extern.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-extern.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - readability-redundant-extern
+
+readability-redundant-extern
+
+
+Removes the redundant or unnecessary ``extern`` keywords from code.
+
+``extern`` is redundant in function declarations, and has no effect in namespaces
+
+The default language linkage is C++, so without any additional parameters it is redundant (extern "C++" can also be redundant, but it depends on the context). In C context (extern "C") the situation is the same, extern keyword is redundant for function declarations
+
+.. code-block:: c++
+
+ extern void h();
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -207,6 +207,11 @@
but either don't specify it or the clause is specified but with the kind
other than ``none``, and suggests to use the ``default(none)`` clause.
+- New :doc:`readability-redundant-extern
+ ` check.
+
+ Removes the redundant ``extern`` keywords.
+
Improvements to clang-include-fixer
---
Index: clang-tidy/readability/RedundantExternCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.h
@@ -0,0 +1,34 @@
+//===--- RedundantExternCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH
[PATCH] D33841: [clang-tidy] redundant keyword check
aaron.ballman added inline comments.
Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);
koldaniel wrote:
> aaron.ballman wrote:
> > koldaniel wrote:
> > > aaron.ballman wrote:
> > > > More tests that I figured out:
> > > > ```
> > > > namespace {
> > > > extern void f(); // 'extern' is not redundant
> > > > }
> > > >
> > > > namespace a {
> > > > namespace {
> > > > namespace b {
> > > > extern void f(); // 'extern' is not redundant
> > > > }
> > > > }
> > > > }
> > > >
> > > > // Note, the above are consequences of
> > > > http://eel.is/c++draft/basic.link#6
> > > >
> > > > #define FOO_EXTERN extern
> > > > typedef int extern_int;
> > > >
> > > > extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we
> > > > don't try to fixit this to be '_int FOO_EXTERN foo();'
> > > >
> > > > // The above is a weird consequence of how specifiers are parsed in C
> > > > and C++
> > > > ```
> > > In the first two examples extern is redundant:
> > >
> > > "An unnamed namespace or a namespace declared directly or indirectly
> > > within an unnamed namespace has internal linkage. All other namespaces
> > > have external linkage."
> > >
> > > Also, based on the examples in http://eel.is/c++draft/basic.link#8 , the
> > > extern keyword has no effect in case of unnamed namespaces. In case of
> > > 'C' linkage defined by an extern keyword the checker does not warn.
> > >
> > > In the first two examples extern is redundant:
> >
> > The `extern` keyword there isn't redundant, it's useless. When I hear
> > "redundant", I read it as "you can remove this keyword because the
> > declaration is already `extern`" but that's not the case there.
> >
> > You are correct that the declarations retain internal linkage -- that's why
> > I think the `extern` keyword isn't redundant so much as it is confusing. We
> > already issue a diagnostic for this in the frontend, so I think we may just
> > want to silence the diagnostic here entirely. https://godbolt.org/z/WE6Fkd
> >
> > WDYT?
> I see, you are right, calling it redundant is not the best way to describe
> the situation. What I wanted to achieve here is that I wanted this checker
> to warn on every occurrence of the keyword extern when it seems to be useless
> (redundant, unnecessary, etc.). If there are other checkers which warn in
> situations like that (i.e. when extern has no effect), we could silence these
> warnings (or we could handle this checker as a more generic one which warns
> not on redundant but on unnecessary uses of extern).
I think it is okay to keep the check as-is but change the wording of the
diagnostic slightly. Maybe `"unnecessary 'extern' keyword; %select{the
declaration already has external linkage|the 'extern' specifier has no
effect}0"` to really make it clear what's going on?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel marked an inline comment as done.
koldaniel added inline comments.
Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);
aaron.ballman wrote:
> koldaniel wrote:
> > aaron.ballman wrote:
> > > More tests that I figured out:
> > > ```
> > > namespace {
> > > extern void f(); // 'extern' is not redundant
> > > }
> > >
> > > namespace a {
> > > namespace {
> > > namespace b {
> > > extern void f(); // 'extern' is not redundant
> > > }
> > > }
> > > }
> > >
> > > // Note, the above are consequences of http://eel.is/c++draft/basic.link#6
> > >
> > > #define FOO_EXTERN extern
> > > typedef int extern_int;
> > >
> > > extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we
> > > don't try to fixit this to be '_int FOO_EXTERN foo();'
> > >
> > > // The above is a weird consequence of how specifiers are parsed in C and
> > > C++
> > > ```
> > In the first two examples extern is redundant:
> >
> > "An unnamed namespace or a namespace declared directly or indirectly within
> > an unnamed namespace has internal linkage. All other namespaces have
> > external linkage."
> >
> > Also, based on the examples in http://eel.is/c++draft/basic.link#8 , the
> > extern keyword has no effect in case of unnamed namespaces. In case of 'C'
> > linkage defined by an extern keyword the checker does not warn.
> >
> > In the first two examples extern is redundant:
>
> The `extern` keyword there isn't redundant, it's useless. When I hear
> "redundant", I read it as "you can remove this keyword because the
> declaration is already `extern`" but that's not the case there.
>
> You are correct that the declarations retain internal linkage -- that's why I
> think the `extern` keyword isn't redundant so much as it is confusing. We
> already issue a diagnostic for this in the frontend, so I think we may just
> want to silence the diagnostic here entirely. https://godbolt.org/z/WE6Fkd
>
> WDYT?
I see, you are right, calling it redundant is not the best way to describe the
situation. What I wanted to achieve here is that I wanted this checker to warn
on every occurrence of the keyword extern when it seems to be useless
(redundant, unnecessary, etc.). If there are other checkers which warn in
situations like that (i.e. when extern has no effect), we could silence these
warnings (or we could handle this checker as a more generic one which warns not
on redundant but on unnecessary uses of extern).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
aaron.ballman added inline comments.
Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);
koldaniel wrote:
> aaron.ballman wrote:
> > More tests that I figured out:
> > ```
> > namespace {
> > extern void f(); // 'extern' is not redundant
> > }
> >
> > namespace a {
> > namespace {
> > namespace b {
> > extern void f(); // 'extern' is not redundant
> > }
> > }
> > }
> >
> > // Note, the above are consequences of http://eel.is/c++draft/basic.link#6
> >
> > #define FOO_EXTERN extern
> > typedef int extern_int;
> >
> > extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we
> > don't try to fixit this to be '_int FOO_EXTERN foo();'
> >
> > // The above is a weird consequence of how specifiers are parsed in C and
> > C++
> > ```
> In the first two examples extern is redundant:
>
> "An unnamed namespace or a namespace declared directly or indirectly within
> an unnamed namespace has internal linkage. All other namespaces have external
> linkage."
>
> Also, based on the examples in http://eel.is/c++draft/basic.link#8 , the
> extern keyword has no effect in case of unnamed namespaces. In case of 'C'
> linkage defined by an extern keyword the checker does not warn.
>
> In the first two examples extern is redundant:
The `extern` keyword there isn't redundant, it's useless. When I hear
"redundant", I read it as "you can remove this keyword because the declaration
is already `extern`" but that's not the case there.
You are correct that the declarations retain internal linkage -- that's why I
think the `extern` keyword isn't redundant so much as it is confusing. We
already issue a diagnostic for this in the frontend, so I think we may just
want to silence the diagnostic here entirely. https://godbolt.org/z/WE6Fkd
WDYT?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel updated this revision to Diff 198194.
koldaniel added a comment.
Typedef and macro maches fixed, new test cases added.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantExternCheck.cpp
clang-tidy/readability/RedundantExternCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/readability-redundant-extern.rst
test/clang-tidy/readability-redundant-extern.cpp
Index: test/clang-tidy/readability-redundant-extern.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-extern.cpp
@@ -0,0 +1,74 @@
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f();
+
+int extern f() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f() { return 0; }
+
+extern "C" int g();
+// CHECK-FIXES: extern "C" int g();
+
+extern "C" int g() { return 0; }
+// CHECK-FIXES: extern "C" int g() { return 0; }
+
+extern "C++" int h();
+// CHECK-FIXES: extern "C++" int h();
+
+extern "C++" int h() { return 0; }
+// CHECK-FIXES: extern "C++" int h() { return 0; }
+
+inline extern void foo_inline();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: inline void foo_inline();
+
+#define FOO_EXTERN extern
+FOO_EXTERN void foo_macro_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_EXTERN void foo_macro_1();
+
+#define FOO_INLINE inline
+FOO_INLINE extern void foo_macro_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_INLINE extern void foo_macro_2();
+
+#define FOO_EXTERN_INLINE inline extern
+FOO_EXTERN_INLINE void foo_macro_3();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: FOO_EXTERN_INLINE void foo_macro_3();
+
+void file_scope();
+// CHECK-FIXES: void file_scope();
+
+void another_file_scope(int _extern);
+// CHECK-FIXES: void another_file_scope(int _extern);
+
+namespace {
+extern void namespace_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: void namespace_1();
+}
+
+namespace a {
+namespace {
+namespace b {
+extern void namespace_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: void namespace_2();
+}
+}
+}
+
+namespace {
+extern "C" void namespace_3();
+// CHECK-FIXES: extern "C" void namespace_3();
+}
+
+#define FOO_EXTERN extern
+typedef int extern_int;
+
+extern_int FOO_EXTERN typedef_extern_foo();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: extern_int FOO_EXTERN typedef_extern_foo();
Index: docs/clang-tidy/checks/readability-redundant-extern.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-extern.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - readability-redundant-extern
+
+readability-redundant-extern
+
+
+Removes the redundant ``extern`` keywords from code.
+
+``extern`` is redundant in function declarations
+
+The default language linkage is C++, so without any additional parameters it is redundant (extern "C++" can also be redundant, but it depends on the context). In C context (extern "C") the situation is the same, extern keyword is redundant for function declarations
+
+.. code-block:: c++
+
+ extern void h();
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -164,6 +164,11 @@
but either don't specify it or the clause is specified but with the kind
other than ``none``, and suggests to use the ``default(none)`` clause.
+- New :doc:`readability-redundant-extern
+ ` check.
+
+ Removes the redundant ``extern`` keywords.
+
Improvements to clang-include-fixer
---
Index: clang-tidy/readability/RedundantExternCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.h
@@ -0,0 +1,34 @@
+//===--- RedundantExternCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel marked an inline comment as done.
koldaniel added inline comments.
Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);
aaron.ballman wrote:
> More tests that I figured out:
> ```
> namespace {
> extern void f(); // 'extern' is not redundant
> }
>
> namespace a {
> namespace {
> namespace b {
> extern void f(); // 'extern' is not redundant
> }
> }
> }
>
> // Note, the above are consequences of http://eel.is/c++draft/basic.link#6
>
> #define FOO_EXTERN extern
> typedef int extern_int;
>
> extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we don't
> try to fixit this to be '_int FOO_EXTERN foo();'
>
> // The above is a weird consequence of how specifiers are parsed in C and C++
> ```
In the first two examples extern is redundant:
"An unnamed namespace or a namespace declared directly or indirectly within an
unnamed namespace has internal linkage. All other namespaces have external
linkage."
Also, based on the examples in http://eel.is/c++draft/basic.link#8 , the extern
keyword has no effect in case of unnamed namespaces. In case of 'C' linkage
defined by an extern keyword the checker does not warn.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
alexfh added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + koldaniel wrote: > alexfh wrote: > > xazax.hun wrote: > > > alexfh wrote: > > > > Could you explain, why you think `extern` is redundant in function > > > > declarations? > > > Just to be clear here, do you think there is a case where extern is not > > > redundant or you just want the documentation to be extended? > > Sorry for being unclear. I would expect a more in-depth explanation of why > > the keyword is redundant with references to the appropriate sections of the > > standard or some other authoritative source. > https://en.cppreference.com/w/cpp/language/language_linkage > > The default language linkage is C++, so without any additional parameters it > is redundant (**extern "C++"** can also be redundant, but it depends on the > context). In C context (**extern "C"**) the situation is the same, **extern** > keyword is redundant for function declarations > (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf - 6.2.2.5) This sort of a description would be helpful in the documentation of the check. Users are not likely to search for clarifications in code review comments on LLVM Phabricator ;) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/ https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:27
+void RedundantExternCheck::check(const MatchFinder::MatchResult &Result) {
+ auto* FD =
+ Result.Nodes.getNodeAs("redundant_extern");
Formatting is incorrect (elsewhere too). You should run the patch through
clang-format.
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:32
+
+ if(!(FD->getStorageClass() == SC_Extern))
+return;
`FD->getStorageClass() != SC_Extern`
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:35
+
+ auto Diag = diag(FD->getBeginLoc(), "redundant 'extern' keyword");
+
How about "redundant 'extern' storage class specifier"?
Comment at: test/clang-tidy/readability-redundant-extern.cpp:37
+
+void another_file_scope(int _extern);
More tests that I figured out:
```
namespace {
extern void f(); // 'extern' is not redundant
}
namespace a {
namespace {
namespace b {
extern void f(); // 'extern' is not redundant
}
}
}
// Note, the above are consequences of http://eel.is/c++draft/basic.link#6
#define FOO_EXTERN extern
typedef int extern_int;
extern_int FOO_EXTERN foo(); // 'extern' is redundant, but hopefully we don't
try to fixit this to be '_int FOO_EXTERN foo();'
// The above is a weird consequence of how specifiers are parsed in C and C++
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel updated this revision to Diff 190377.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantExternCheck.cpp
clang-tidy/readability/RedundantExternCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-redundant-extern.rst
test/clang-tidy/readability-redundant-extern.cpp
Index: test/clang-tidy/readability-redundant-extern.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-extern.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f();
+
+extern int f() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f() { return 0; }
+
+extern "C" int g();
+
+extern "C" int g() { return 0; }
+
+extern "C++" int h();
+
+extern "C++" int h() { return 0; }
+
+inline extern void foo_inline();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: inline void foo_inline();
+
+#define FOO_EXTERN extern
+FOO_EXTERN void foo_macro_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_INLINE inline
+FOO_INLINE extern void foo_macro_3();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_EXTERN_INLINE inline extern
+FOO_EXTERN_INLINE void foo_macro_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+void file_scope();
+
+void another_file_scope(int _extern);
Index: docs/clang-tidy/checks/readability-redundant-extern.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-extern.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-redundant-extern
+
+readability-redundant-extern
+
+
+Removes the redundant ``extern`` keywords from code.
+
+``extern`` is redundant in function declarations
+
+.. code-block:: c++
+
+ extern void h();
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -258,6 +258,7 @@
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
+ readability-redundant-extern
readability-redundant-function-ptr-dereference
readability-redundant-member-init
readability-redundant-preprocessor
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -117,6 +117,11 @@
` now supports `OverrideSpelling`
and `FinalSpelling` options.
+- New :doc:`readability-redundant-extern
+ ` check.
+
+ Removes the redundant ``extern`` keywords.
+
Improvements to include-fixer
-
Index: clang-tidy/readability/RedundantExternCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.h
@@ -0,0 +1,34 @@
+//===--- RedundantExternCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds redundant 'extern' keywords
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-extern.html
+class RedundantExternCheck : public ClangTidyCheck {
+public:
+ RedundantExternCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
Index: clang-tidy/readability/RedundantExternCheck.cpp
===
--- /dev/null
+++ clang-tidy/readabilit
[PATCH] D33841: [clang-tidy] redundant keyword check
aaron.ballman added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:4 +readability-redundant-extern += + The underlining here is too long. Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:14 + \ No newline at end of file Please add the newline to the end of the file. Comment at: test/clang-tidy/readability-redundant-extern.cpp:2 +// RUN: %check_clang_tidy %s readability-redundant-extern %t + +extern int f(); This is missing some other negative tests, like a file-scope: ``` void file_scope(); ``` an especially interesting test would be: ``` void another_file_scope(int _extern); ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/ https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
MyDeveloperDay added inline comments.
Comment at: clang-tidy/readability/RedundantExternCheck.cpp:45
+
+ int offset = Text.find("extern");
+
current convention would be that this should be Offset
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
Eugene.Zelenko added a comment. Please mention new check in Release Notes. Comment at: clang-tidy/readability/RedundantExternCheck.cpp:55 +} // namespace clang \ No newline at end of file Please add new line. Comment at: clang-tidy/readability/RedundantExternCheck.h:5 +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. Please update license statement. See current Clang-tidy code as example. Same in other files. Comment at: docs/clang-tidy/checks/readability-redundant-extern.rst:6 + +This checker removes the redundant `extern` keywords from code. + Please remove //This checker//. Language constructs should be enclosed in ``. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/ https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel updated this revision to Diff 186132.
koldaniel added a comment.
Rebased.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantExternCheck.cpp
clang-tidy/readability/RedundantExternCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-redundant-extern.rst
test/clang-tidy/readability-redundant-extern.cpp
Index: test/clang-tidy/readability-redundant-extern.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-extern.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f();
+
+extern int f() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f() { return 0; }
+
+extern "C" int g();
+
+extern "C" int g() { return 0; }
+
+extern "C++" int h();
+
+extern "C++" int h() { return 0; }
+
+inline extern void foo_inline();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: inline void foo_inline();
+
+#define FOO_EXTERN extern
+FOO_EXTERN void foo_macro_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_INLINE inline
+FOO_INLINE extern void foo_macro_3();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_EXTERN_INLINE inline extern
+FOO_EXTERN_INLINE void foo_macro_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
Index: docs/clang-tidy/checks/readability-redundant-extern.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-extern.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-redundant-extern
+
+readability-redundant-extern
+=
+
+This checker removes the redundant `extern` keywords from code.
+
+`extern` is redundant in function declarations
+
+.. code-block:: c++
+
+ extern void h();
+
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -256,6 +256,7 @@
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
+ readability-redundant-extern
readability-redundant-function-ptr-dereference
readability-redundant-member-init
readability-redundant-preprocessor
Index: clang-tidy/readability/RedundantExternCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantExternCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds redundant 'extern' keywords
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-extern.html
+class RedundantExternCheck : public ClangTidyCheck {
+public:
+ RedundantExternCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
Index: clang-tidy/readability/RedundantExternCheck.cpp
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.cpp
@@ -0,0 +1,54 @@
+//===--- RedundantExternCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantExternCheck.h"
+#include "clang/
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel updated this revision to Diff 184111.
koldaniel added a comment.
As it was mentioned earlier, I think it would be a better way forward to handle
the check of redundant inlines in the scope of the other checker:
https://reviews.llvm.org/D18914
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D33841/new/
https://reviews.llvm.org/D33841
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/RedundantExternCheck.cpp
clang-tidy/readability/RedundantExternCheck.h
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-redundant-extern.rst
test/clang-tidy/readability-redundant-extern.cpp
Index: test/clang-tidy/readability-redundant-extern.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-extern.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s readability-redundant-extern %t
+
+extern int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f();
+
+extern int f() { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: int f() { return 0; }
+
+extern "C" int g();
+
+extern "C" int g() { return 0; }
+
+extern "C++" int h();
+
+extern "C++" int h() { return 0; }
+
+inline extern void foo_inline();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+// CHECK-FIXES: inline void foo_inline();
+
+#define FOO_EXTERN extern
+FOO_EXTERN void foo_macro_1();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_INLINE inline
+FOO_INLINE extern void foo_macro_3();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
+
+#define FOO_EXTERN_INLINE inline extern
+FOO_EXTERN_INLINE void foo_macro_2();
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant 'extern' keyword [readability-redundant-extern]
Index: docs/clang-tidy/checks/readability-redundant-extern.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-extern.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - readability-redundant-extern
+
+readability-redundant-extern
+=
+
+This checker removes the redundant `extern` keywords from code.
+
+`extern` is redundant in function declarations
+
+.. code-block:: c++
+
+ extern void h();
+
\ No newline at end of file
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -224,6 +224,7 @@
readability-non-const-parameter
readability-redundant-control-flow
readability-redundant-declaration
+ readability-redundant-extern
readability-redundant-function-ptr-dereference
readability-redundant-member-init
readability-redundant-smartptr-get
Index: clang-tidy/readability/RedundantExternCheck.h
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.h
@@ -0,0 +1,35 @@
+//===--- RedundantExternCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds redundant 'extern' keywords
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-extern.html
+class RedundantExternCheck : public ClangTidyCheck {
+public:
+ RedundantExternCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANT_EXTERN_H
Index: clang-tidy/readability/RedundantExternCheck.cpp
===
--- /dev/null
+++ clang-tidy/readability/RedundantExternCheck.cpp
@@ -0,0 +1,54 @@
+//===--- RedundantExternCheck.cpp - clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License
[PATCH] D33841: [clang-tidy] redundant keyword check
koldaniel marked an inline comment as done. koldaniel added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + alexfh wrote: > xazax.hun wrote: > > alexfh wrote: > > > Could you explain, why you think `extern` is redundant in function > > > declarations? > > Just to be clear here, do you think there is a case where extern is not > > redundant or you just want the documentation to be extended? > Sorry for being unclear. I would expect a more in-depth explanation of why > the keyword is redundant with references to the appropriate sections of the > standard or some other authoritative source. https://en.cppreference.com/w/cpp/language/language_linkage The default language linkage is C++, so without any additional parameters it is redundant (**extern "C++"** can also be redundant, but it depends on the context). In C context (**extern "C"**) the situation is the same, **extern** keyword is redundant for function declarations (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf - 6.2.2.5) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/ https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
alexfh added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + xazax.hun wrote: > alexfh wrote: > > Could you explain, why you think `extern` is redundant in function > > declarations? > Just to be clear here, do you think there is a case where extern is not > redundant or you just want the documentation to be extended? Sorry for being unclear. I would expect a more in-depth explanation of why the keyword is redundant with references to the appropriate sections of the standard or some other authoritative source. Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
xazax.hun added inline comments. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + alexfh wrote: > Could you explain, why you think `extern` is redundant in function > declarations? Just to be clear here, do you think there is a case where extern is not redundant or you just want the documentation to be extended? Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
aaron.ballman added inline comments.
Comment at: clang-tidy/readability/RedundantKeywordCheck.cpp:22
+template
+static bool startsWith(const T &Node, StringRef Value) {
+ Token Result;
Why do you need to do a textual search that the first token in the declaration
is extern or inline? That seems like it would fail on common cases that you
would expect to catch:
```
#define FOO_EXTERN extern
FOO_EXTERN void blah();
```
Comment at: test/clang-tidy/readability-redundant-keyword.cpp:22
+
+extern "C" void ok();
+
Why is this okay, but the following is not?
```
extern "C++" void ok2();
```
Repository:
rL LLVM
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:8 + +`extern` is redundant in function declarations + Could you explain, why you think `extern` is redundant in function declarations? Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
Prazek added a comment. extern on function definition is also redundant, right? Also, what about: inline extern void foo(); (you check if it startswith extern) Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
Eugene.Zelenko added a comment. In https://reviews.llvm.org/D33841#771944, @Prazek wrote: > Feature request: removing "void" from int main(void) This will duplicate modernize-redundant-void-arg. Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
Prazek added a comment. Feature request: removing "void" from int main(void) Repository: rL LLVM https://reviews.llvm.org/D33841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33841: [clang-tidy] redundant keyword check
Eugene.Zelenko added a comment.
Please mention this check in docs/ReleaseNotes.rst (in alphabetical order).
Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:6
+
+This checker removes the redundant `extern` and `inline` keywords from code.
+
checker -> check. Please use `` to highlight language constructs here and below.
Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:13
+ extern void h();
+
+
Unnecessary empty line.
Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:20
+ class X {
+inline int f() { }
+ };
May be three dots will be better as common punctuation?
Comment at: docs/clang-tidy/checks/readability-redundant-keyword.rst:22
+ };
\ No newline at end of file
Please add newline.
Repository:
rL LLVM
https://reviews.llvm.org/D33841
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
