summaryrefslogtreecommitdiff
path: root/compilerplugins
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2020-07-02 21:12:04 +0200
committerStephan Bergmann <sbergman@redhat.com>2020-07-02 23:27:05 +0200
commitf4c126da920b06d273ddd375d7f77faa39f01cb5 (patch)
treee891d2fa5ff05e90faa37f3eac6356adbf899a83 /compilerplugins
parent45504ca72e0f081de9e7cc4f1faf553c7361694d (diff)
Improved loplugin:staticanonymous -> redundantstatic
...now also covering variables with internal linkage that don't need a redundant "static". (Unlike with functions, with variables there are also cases that are not in an unnamed namespace, hence the rename of the plugin.) All the relevant changes across the code base have been done in the preceding "Upcoming improved loplugin:staticanonymous -> redundantstatic" commits. Ideally the changes would have been done with a rewriting plugin, but it can be quite tedious in general to identify the correct occurrence of "static" that must be removed, consider e.g. struct { int init() { static int n; return n++; } int x = init(); } static const a[10] = {}; However, it turned out that in all cases across the code base, the relevant "static" was either at the start of the declaration or came after an initial "const". So I temporarily changed the plugin with > --- a/compilerplugins/clang/redundantstatic.cxx > +++ b/compilerplugins/clang/redundantstatic.cxx > @@ -59,7 +59,7 @@ class RedundantStatic > } > report( > DiagnosticsEngine::Warning, "redundant 'static' keyword in unnamed namespace", > - decl->getLocation()) > + decl->getBeginLoc()) > << decl->getSourceRange(); > return true; > } > @@ -73,7 +73,7 @@ class RedundantStatic > DiagnosticsEngine::Warning, > "non-inline variable of non-volatile const-qualified type is redundantly marked as" > " 'static'", > - decl->getLocation()) > + decl->getBeginLoc()) > << decl->getSourceRange(); > return true; > } to report the diagnostics at the start of the declarations (instead of at a more natural place which is typically somewhere in the middle of the declaration), compiled LO from within Emacs and then ran a function > (defun doit () > (interactive) > (while t > (next-error) > (with-current-buffer (window-buffer) > (when (re-search-forward > "\\=\\(\\<static\\>\\s *\\|\\(\\<const\\>\\)\\s +\\<static\\>\\)" > nil t) > (replace-match "\\2"))))) to do all the replacements. (Plus solenv/clang-format/reformat-formatted-files where necessary.) Change-Id: Ie7efc8e0593a407c390a6a7a08c81e547410f18a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97779 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'compilerplugins')
-rw-r--r--compilerplugins/clang/check.cxx10
-rw-r--r--compilerplugins/clang/check.hxx2
-rw-r--r--compilerplugins/clang/redundantstatic.cxx123
-rw-r--r--compilerplugins/clang/staticanonymous.cxx70
4 files changed, 135 insertions, 70 deletions
diff --git a/compilerplugins/clang/check.cxx b/compilerplugins/clang/check.cxx
index b4317c07bcf7..2ae58504fe3b 100644
--- a/compilerplugins/clang/check.cxx
+++ b/compilerplugins/clang/check.cxx
@@ -64,6 +64,16 @@ TypeCheck TypeCheck::ConstVolatile() const {
// checking for
}
+TypeCheck TypeCheck::ConstNonVolatile() const {
+ return
+ (!type_.isNull() && type_.isConstQualified()
+ && !type_.isVolatileQualified())
+ ? *this : TypeCheck();
+ // returning TypeCheck(type_.getUnqualifiedType()) instead of *this
+ // may look tempting, but could remove sugar we might be interested in
+ // checking for
+}
+
TerminalCheck TypeCheck::Void() const {
return TerminalCheck(
!type_.isNull()
diff --git a/compilerplugins/clang/check.hxx b/compilerplugins/clang/check.hxx
index 8e835b8b6a96..f8793e14216c 100644
--- a/compilerplugins/clang/check.hxx
+++ b/compilerplugins/clang/check.hxx
@@ -51,6 +51,8 @@ public:
TypeCheck ConstVolatile() const;
+ TypeCheck ConstNonVolatile() const;
+
TerminalCheck Void() const;
TerminalCheck Char() const;
diff --git a/compilerplugins/clang/redundantstatic.cxx b/compilerplugins/clang/redundantstatic.cxx
new file mode 100644
index 000000000000..95a3c85c9d17
--- /dev/null
+++ b/compilerplugins/clang/redundantstatic.cxx
@@ -0,0 +1,123 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * Based on LLVM/Clang.
+ *
+ */
+#ifndef LO_CLANG_SHARED_PLUGINS
+
+#include "check.hxx"
+#include "compat.hxx"
+#include "plugin.hxx"
+
+/*
+This is a compile check.
+
+Warns about functions with static keyword in an unnamed namespace.
+*/
+
+namespace loplugin
+{
+
+class RedundantStatic
+ : public loplugin::FilteringPlugin<RedundantStatic>
+ {
+ public:
+ explicit RedundantStatic( const InstantiationData& data );
+
+ bool preRun() override { return compiler.getLangOpts().CPlusPlus; }
+
+ virtual void run() override;
+ bool VisitFunctionDecl( const FunctionDecl* func );
+
+ bool VisitVarDecl(VarDecl const * decl) {
+ if (ignoreLocation(decl)) {
+ return true;
+ }
+ if (!decl->isFileVarDecl() || decl->isStaticDataMember()) {
+ return true;
+ }
+ if (decl->getStorageClass() != SC_Static) {
+ return true;
+ }
+ if (decl->isInAnonymousNamespace()) {
+ auto loc = compat::getBeginLoc(decl);
+ while (compiler.getSourceManager().isMacroArgExpansion(loc)) {
+ loc = compiler.getSourceManager().getImmediateMacroCallerLoc(loc);
+ }
+ if (compiler.getSourceManager().isMacroBodyExpansion(loc)) {
+ auto const name = Lexer::getImmediateMacroName(
+ loc, compiler.getSourceManager(), compiler.getLangOpts());
+ if (name == "CPPUNIT_TEST_SUITE_REGISTRATION"
+ || name == "CPPUNIT_TEST_SUITE_NAMED_REGISTRATION")
+ {
+ // Those macros contain a `static`, but are often used in an unnamed
+ // namespace, so filter them out:
+ return true;
+ }
+ }
+ report(
+ DiagnosticsEngine::Warning, "redundant 'static' keyword in unnamed namespace",
+ decl->getLocation())
+ << decl->getSourceRange();
+ return true;
+ }
+ if (decl->isInline()) {
+ return true;
+ }
+ if (!loplugin::TypeCheck(decl->getType()).ConstNonVolatile()) {
+ return true;
+ }
+ report(
+ DiagnosticsEngine::Warning,
+ "non-inline variable of non-volatile const-qualified type is redundantly marked as"
+ " 'static'",
+ decl->getLocation())
+ << decl->getSourceRange();
+ return true;
+ }
+ };
+
+RedundantStatic::RedundantStatic( const InstantiationData& data )
+ : FilteringPlugin( data )
+ {
+ }
+
+void RedundantStatic::run()
+ {
+ if (preRun()) {
+ TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
+ }
+ }
+
+
+bool RedundantStatic::VisitFunctionDecl( const FunctionDecl* func )
+
+ {
+ if( ignoreLocation( func ) )
+ return true;
+ if( func -> isInAnonymousNamespace () )
+ {
+ if ( !isa<CXXMethodDecl>(func) && !func->isInExternCContext() )
+ {
+ if(func-> getStorageClass() == SC_Static)
+ {
+ report( DiagnosticsEngine::Warning,
+ "redundant 'static' keyword in unnamed namespace",
+ compat::getBeginLoc(func));
+ }
+ }
+ }
+
+ return true;
+ }
+
+// Register the plugin action with the LO plugin handling.
+static Plugin::Registration< RedundantStatic > redundantstatic("redundantstatic");
+
+} // namespace
+
+#endif // LO_CLANG_SHARED_PLUGINS
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/compilerplugins/clang/staticanonymous.cxx b/compilerplugins/clang/staticanonymous.cxx
deleted file mode 100644
index 0fd00f2279e7..000000000000
--- a/compilerplugins/clang/staticanonymous.cxx
+++ /dev/null
@@ -1,70 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * Based on LLVM/Clang.
- *
- */
-#ifndef LO_CLANG_SHARED_PLUGINS
-
-#include "plugin.hxx"
-
-/*
-This is a compile check.
-
-Warns about functions with static keyword in an unnamed namespace.
-*/
-
-namespace loplugin
-{
-
-class StaticAnonymous
- : public loplugin::FilteringPlugin<StaticAnonymous>
- {
- public:
- explicit StaticAnonymous( const InstantiationData& data );
- virtual void run() override;
- bool VisitFunctionDecl( const FunctionDecl* func );
-
- };
-
-StaticAnonymous::StaticAnonymous( const InstantiationData& data )
- : FilteringPlugin( data )
- {
- }
-
-void StaticAnonymous::run()
- {
- TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
- }
-
-
-bool StaticAnonymous::VisitFunctionDecl( const FunctionDecl* func )
-
- {
- if( ignoreLocation( func ) )
- return true;
- if( func -> isInAnonymousNamespace () )
- {
- if ( !isa<CXXMethodDecl>(func) && !func->isInExternCContext() )
- {
- if(func-> getStorageClass() == SC_Static)
- {
- report( DiagnosticsEngine::Warning,
- "redundant 'static' keyword in unnamed namespace",
- compat::getBeginLoc(func));
- }
- }
- }
-
- return true;
- }
-
-// Register the plugin action with the LO plugin handling.
-static Plugin::Registration< StaticAnonymous > staticanonymous("staticanonymous");
-
-} // namespace
-
-#endif // LO_CLANG_SHARED_PLUGINS
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab: */