loplugin:externvar is covered by loplugin:external

...so drop the former.  But keep the relevant externvar tests by moving them
into compilerplugins/clang/test/external.cxx.  (Which revealed one difference
between the two plugins, regarding certain extern "C" variables in unnamed
namespaces, where Clang (and for that matter also e.g. GCC, it appears)
deliberately deviates from the Standard and considers them to have external
linkage.  Add clarifying comments that loplugin:external keeps considering these
as having internal linkage, following the Standard.)

Change-Id: I344fcd0135fdaf6bf08a4b396af2ed2299389a7d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97639
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

7 files changed, 84 insertions, 167 deletions

diff --git a/compilerplugins/clang/external.cxx b/compilerplugins/clang/external.cxx
index 6ec2cfdb1c8d..bc75237a15fd 100644
--- a/compilerplugins/clang/external.cxx
+++ b/compilerplugins/clang/external.cxx
@@ -471,8 +471,17 @@ private:
         {
             return true;
         }
-        //TODO: in some cases getLinkageInternal() appears to report ExternalLinkage instead of
-        // UniqueExternalLinkage:
+        // In some cases getLinkageInternal() arguably wrongly reports ExternalLinkage, see the
+        // commit message of <https://github.com/llvm/llvm-project/commit/
+        // df963a38a9e27fc43b485dfdf52bc1b090087e06> "DR1113: anonymous namespaces formally give
+        // their contents internal linkage":
+        //
+        //  "We still deviate from the standard in one regard here: extern "C" declarations
+        //   in anonymous namespaces are still granted external linkage. Changing those does
+        //   not appear to have been an intentional consequence of the standard change in
+        //   DR1113."
+        //
+        // Do not warn about such "wrongly external" declarations here:
         if (decl->isInAnonymousNamespace())
         {
             return true;
diff --git a/compilerplugins/clang/externvar.cxx b/compilerplugins/clang/externvar.cxx
deleted file mode 100644
index 05e9820d4da3..000000000000
--- a/compilerplugins/clang/externvar.cxx
+++ /dev/null
@@ -1,95 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
-#ifndef LO_CLANG_SHARED_PLUGINS
-
-#include "check.hxx"
-#include "plugin.hxx"
-
-// Find variable declarations at namespace scope that need not have external
-// linkage.
-
-namespace {
-
-class ExternVar: public loplugin::FilteringPlugin<ExternVar>
-{
-public:
-    explicit ExternVar(loplugin::InstantiationData const & data): FilteringPlugin(data)
-    {}
-
-    void run() override
-    { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
-
-    bool VisitVarDecl(VarDecl const * decl) {
-        if (ignoreLocation(decl)) {
-            return true;
-        }
-        if (decl->isStaticDataMember()) {
-            return true;
-        }
-        if (!(decl->isFirstDecl()
-              && compiler.getSourceManager().isInMainFile(decl->getLocation())
-              && loplugin::hasExternalLinkage(decl)))
-        {
-            return true;
-        }
-        auto def = decl->getDefinition();
-        if (def == nullptr) {
-            // Code like
-            //
-            //   namespace { extern int v; }
-            //   int f() { return sizeof(v); }
-            //
-            // is already handled by Clang itself with an error "variable 'v' is
-            // not needed and will not be emitted"
-            return true;
-        }
-        if (loplugin::DeclCheck(def).Var("_pRawDllMain").GlobalNamespace()) {
-            return true;
-        }
-        SourceLocation argLoc;
-        if (compiler.getSourceManager().isMacroArgExpansion(def->getLocation(), &argLoc)
-            && (Lexer::getImmediateMacroName(
-                    argLoc, compiler.getSourceManager(), compiler.getLangOpts())
-                == "DEFINE_GUID"))
-        {
-            return true;
-        }
-        report(
-            DiagnosticsEngine::Warning,
-            "variable with external linkage not declared in an include file",
-            def->getLocation())
-            << def->getSourceRange();
-        report(
-            DiagnosticsEngine::Note,
-            ("should either have internal linkage or be declared in an include"
-             " file"),
-            def->getLocation())
-            << def->getSourceRange();
-        for (auto prev = def;;) {
-            prev = prev->getPreviousDecl();
-            if (prev == nullptr) {
-                break;
-            }
-            report(
-                DiagnosticsEngine::Note, "previously declared here",
-                prev->getLocation())
-                << prev->getSourceRange();
-        }
-        return true;
-    }
-};
-
-loplugin::Plugin::Registration<ExternVar> externvar("externvar");
-
-}
-
-#endif // LO_CLANG_SHARED_PLUGINS
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/external.cxx b/compilerplugins/clang/test/external.cxx
index 6eb486a57fc1..d0391f0cd612 100644
--- a/compilerplugins/clang/test/external.cxx
+++ b/compilerplugins/clang/test/external.cxx
@@ -11,6 +11,10 @@
 
 #include <vector>
 
+#include "external.hxx"
+
+int n0; // no warning, see external.hxx
+
 // expected-error@+1 {{externally available entity 'n1' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
 int n1 = 0;
 // expected-note@+1 {{another declaration is here [loplugin:external]}}
@@ -20,6 +24,68 @@ int const n2 = 0; // no warning, internal linkage
 
 constexpr int n3 = 0; // no warning, internal linkage
 
+static int n4; // no warning, internal linkage
+
+// expected-note@+1 {{another declaration is here [loplugin:external]}}
+extern int n5;
+// expected-error@+1 {{externally available entity 'n5' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
+int n5;
+
+// expected-note@+1 {{another declaration is here [loplugin:external]}}
+extern "C" int n6;
+// expected-error@+1 {{externally available entity 'n6' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
+int n6;
+
+extern "C" {
+// expected-error@+1 {{externally available entity 'n7' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
+int n7;
+}
+
+namespace
+{
+int u1; // no warning, internal linkage
+
+static int u2; // no warning, internal linkage
+
+extern "C" int u3;
+int u3; // no warning, see the comment about DR1113 in compilerplugins/clang/external.cxx
+
+extern "C" {
+int u4; // no warning, internal linkage
+}
+}
+
+namespace N
+{
+int v1; // no warning, see external.hxx
+
+// expected-error@+1 {{externally available entity 'v2' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
+int v2;
+
+static int v3; // no warning, internal linkage
+}
+
+struct S
+{
+    static int f()
+    {
+        static int s = 0;
+        return s;
+    }
+
+    static int m;
+};
+
+int S::m = 0; // no warning
+
+int f(int a) // no warning about parameters
+{
+    static int s = 0; // no warning about local static variables
+    ++s;
+    int b = a + s; // no warning about local variables
+    return b;
+}
+
 // expected-error@+1 {{externally available entity 'S1' is not previously declared in an included file (if it is only used in this translation unit, put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
 struct S1
 {
@@ -118,6 +184,10 @@ int main()
 {
     (void)n2;
     (void)n3;
+    (void)n4;
+    (void)u1;
+    (void)u2;
+    (void)N::v3;
     g();
     (void)&N::g;
 }
diff --git a/compilerplugins/clang/test/externvar.hxx b/compilerplugins/clang/test/external.hxx
index 225f1ee59fd0..749fec0ee3c7 100644
--- a/compilerplugins/clang/test/externvar.hxx
+++ b/compilerplugins/clang/test/external.hxx
@@ -7,10 +7,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-#ifndef INClUDED_COMPILERPLUGINS_CLANG_TEST_EXTERNVAR_HXX
-#define INClUDED_COMPILERPLUGINS_CLANG_TEST_EXTERNVAR_HXX
+#pragma once
 
-extern int v1;
+extern int n0;
 
 namespace N {
 
@@ -22,6 +21,4 @@ struct S;
 
 int f(int a);
 
-#endif
-
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/test/externvar.cxx b/compilerplugins/clang/test/externvar.cxx
deleted file mode 100644
index c4b30d6625f0..000000000000
--- a/compilerplugins/clang/test/externvar.cxx
+++ /dev/null
@@ -1,61 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
-#include "externvar.hxx"
-
-int v1;
-int v2; // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-static int v3;
-int const v4 = 0;
-
-extern int v5; // expected-note {{previously declared here [loplugin:externvar]}}
-int v5; // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-
-extern "C" int v6; // expected-note {{previously declared here [loplugin:externvar]}}
-int v6; // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-extern "C" { int v7; } // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-
-namespace {
-
-int u1;
-static int u2;
-extern "C" int u3; // expected-note {{previously declared here [loplugin:externvar]}}
-int u3; // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-extern "C" { int u4; }
-
-}
-
-namespace N {
-
-int v1;
-int v2; // expected-error {{variable with external linkage not declared in an include file [loplugin:externvar]}} expected-note {{should either have internal linkage or be declared in an include file [loplugin:externvar]}}
-static int v3;
-
-}
-
-struct S {
-    static int f() {
-        static int s = 0;
-        return s;
-    }
-
-    static int m;
-};
-
-int S::m = 0;
-
-int f(int a) {
-    static int s = 0;
-    ++s;
-    int b = a + s + v1 + v2 + v3 + v4 + v5 + v6 + v7 + u1 + u2 + u3 + u4 + N::v1
-        + N::v2 + N::v3 + S::f();
-    return b;
-}
-
-/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 3d457af768df..d4612b628838 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -35,7 +35,6 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/emptyif \
     compilerplugins/clang/test/expressionalwayszero \
     compilerplugins/clang/test/external \
-    compilerplugins/clang/test/externvar \
     compilerplugins/clang/test/faileddyncast \
     compilerplugins/clang/test/fakebool \
     compilerplugins/clang/test/finalprotected \
diff --git a/solenv/clang-format/blacklist b/solenv/clang-format/blacklist
index 2ffa91497589..52d307c81f1b 100644
--- a/solenv/clang-format/blacklist
+++ b/solenv/clang-format/blacklist
@@ -1652,7 +1652,6 @@ compilerplugins/clang/dynexcspec.cxx
 compilerplugins/clang/expandablemethods.cxx
 compilerplugins/clang/expressionalwayszero.cxx
 compilerplugins/clang/externandnotdefined.cxx
-compilerplugins/clang/externvar.cxx
 compilerplugins/clang/faileddyncast.cxx
 compilerplugins/clang/fakebool.cxx
 compilerplugins/clang/finalclasses.cxx
@@ -1760,8 +1759,7 @@ compilerplugins/clang/test/cppunitassertequals.hxx
 compilerplugins/clang/test/datamembershadow.cxx
 compilerplugins/clang/test/dodgyswitch.cxx
 compilerplugins/clang/test/expressionalwayszero.cxx
-compilerplugins/clang/test/externvar.cxx
-compilerplugins/clang/test/externvar.hxx
+compilerplugins/clang/test/external.hxx
 compilerplugins/clang/test/faileddyncast.cxx
 compilerplugins/clang/test/fakebool.cxx
 compilerplugins/clang/test/finalprotected.cxx