check for unnecessary DLLPUBLIC annotations

on classes which are fully defined in a header file

Rename the dllprivate plugin to dllmacro and add the functionality
there.

Change-Id: I4581d551c46a8f61213d95973f323359d08278d8

chart2/source/inc/CommonFunctors.hxx

@@ -50,7 +50,7 @@ template< typename T >
     <p>In case no number can be generated from the Any, NaN (see
     rtl::math::SetNAN()) is returned.</p>
 */
-struct OOO_DLLPUBLIC_CHARTTOOLS AnyToDouble
+struct AnyToDouble
 {
     double operator() ( const css::uno::Any & rAny )
     {
@@ -64,7 +64,7 @@ struct OOO_DLLPUBLIC_CHARTTOOLS AnyToDouble
 /** unary function to convert css::uno::Any into an
     OUString.
 */
-struct OOO_DLLPUBLIC_CHARTTOOLS AnyToString
+struct AnyToString
 {
     OUString operator() ( const css::uno::Any & rAny )
     {
@@ -93,7 +93,7 @@ struct OOO_DLLPUBLIC_CHARTTOOLS AnyToString
 
     <p>For conversion rtl::math::StringToDouble is used.</p>
  */
-struct OOO_DLLPUBLIC_CHARTTOOLS OUStringToDouble
+struct OUStringToDouble
 {
     double operator() ( const OUString & rStr )
     {
@@ -111,7 +111,7 @@ struct OOO_DLLPUBLIC_CHARTTOOLS OUStringToDouble
 
     <p>For conversion rtl::math::DoubleToOUString is used.</p>
  */
-struct OOO_DLLPUBLIC_CHARTTOOLS DoubleToOUString
+struct DoubleToOUString
 {
     OUString operator() ( double fNumber )
     {

chart2/source/inc/PropertyHelper.hxx

@@ -138,7 +138,7 @@ OOO_DLLPUBLIC_CHARTTOOLS void setEmptyPropertyValueDefault( tPropertyValueMap &
 
 } // namespace PropertyHelper
 
-struct OOO_DLLPUBLIC_CHARTTOOLS PropertyNameLess
+struct PropertyNameLess
 {
     bool operator() ( const css::beans::Property & first,
                              const css::beans::Property & second )

compilerplugins/clang/dllprivate.cxx → compilerplugins/clang/dllmacro.cxx

@@ -8,32 +8,54 @@
  */
 
 #include "plugin.hxx"
+#include <iostream>
+#include <fstream>
 
-namespace {
-
-class Visitor final:
-    public RecursiveASTVisitor<Visitor>, public loplugin::Plugin
+/**
+  Check the usage of the various DLL visibility macros.
+ */
+class DllMacro final:
+    public RecursiveASTVisitor<DllMacro>, public loplugin::Plugin
 {
 public:
-    explicit Visitor(InstantiationData const & data): Plugin(data) {}
+    explicit DllMacro(InstantiationData const & data): Plugin(data) {}
 
-    bool VisitNamedDecl(NamedDecl const * decl) {
-        if (!decl->getLocation().isInvalid()&&ignoreLocation(decl)) {
-            return true;
-        }
-        auto a = decl->getAttr<VisibilityAttr>();
-        if (a == nullptr || a->getVisibility() != VisibilityAttr::Hidden) {
-            return true;
-        }
-        if (compiler.getSourceManager().isMacroBodyExpansion(
-                decl->getLocation())
-            && (Lexer::getImmediateMacroName(
-                    decl->getLocation(), compiler.getSourceManager(),
-                    compiler.getLangOpts())
-                == "Q_OBJECT")) // from /usr/include/QtCore/qobjectdefs.h
+    bool VisitNamedDecl(NamedDecl const * decl);
+
+private:
+    void run() override {
+        // DISABLE_DYNLOADING makes SAL_DLLPUBLIC_{EXPORT,IMPORT,TEMPLATE} expand
+        // to visibility("hidden") attributes, which would cause bogus warnings
+        // here (e.g., in UBSan builds that explicitly define DISABLE_DYNLOADING
+        // in jurt/source/pipe/staticsalhack.cxx); alternatively, change
+        // include/sal/types.h to make those SAL_DLLPUBLIC_* expand to nothing
+        // for DISABLE_DYNLOADING:
+        if (!compiler.getPreprocessor().getIdentifierInfo("DISABLE_DYNLOADING")
+            ->hasMacroDefinition())
         {
-            return true;
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
         }
+    }
+};
+
+bool DllMacro::VisitNamedDecl(NamedDecl const * decl) {
+    if (!decl->getLocation().isInvalid() && ignoreLocation(decl)) {
+        return true;
+    }
+    auto a = decl->getAttr<VisibilityAttr>();
+    if (a == nullptr) {
+        return true;
+    }
+    if (compiler.getSourceManager().isMacroBodyExpansion(
+            decl->getLocation())
+        && (Lexer::getImmediateMacroName(
+                decl->getLocation(), compiler.getSourceManager(),
+                compiler.getLangOpts())
+            == "Q_OBJECT")) // from /usr/include/QtCore/qobjectdefs.h
+    {
+        return true;
+    }
+    if (a->getVisibility() == VisibilityAttr::Hidden) {
         auto p = dyn_cast<RecordDecl>(decl->getDeclContext());
         if (p == nullptr) {
             report(
@@ -45,7 +67,7 @@ public:
             report(
                 DiagnosticsEngine::Warning,
                 ("declaration nested in DLLPRIVATE declaration redundantly"
-                 " marked as DLLPRIVATE"),
+                  " marked as DLLPRIVATE"),
                 a->getLocation())
                 << decl->getSourceRange();
             report(
@@ -53,27 +75,37 @@ public:
                 p->getLocation())
                 << p->getSourceRange();
         }
-        return true;
     }
-
-private:
-    void run() override {
-        // DISABLE_DYNLOADING makes SAL_DLLPUBLIC_{EXPORT,IMPORT,TEMPLATE} expand
-        // to visibility("hidden") attributes, which would cause bogus warnings
-        // here (e.g., in UBSan builds that explicitly define DISABLE_DYNLOADING
-        // in jurt/source/pipe/staticsalhack.cxx); alternatively, change
-        // include/sal/types.h to make those SAL_DLLPUBLIC_* expand to nothing
-        // for DISABLE_DYNLOADING:
-        if (!compiler.getPreprocessor().getIdentifierInfo("DISABLE_DYNLOADING")
-            ->hasMacroDefinition())
-        {
-            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+    else if (a->getVisibility() == VisibilityAttr::Default) {
+        auto p = dyn_cast<CXXRecordDecl>(decl);
+        if (p && p->isCompleteDefinition() && !p->getDescribedClassTemplate()) {
+            // don't know what these macros mean, leave them alone
+            auto macroLoc = a->getLocation();
+            while (compiler.getSourceManager().isMacroBodyExpansion(macroLoc)) {
+                auto macroName = Lexer::getImmediateMacroName(
+                        macroLoc, compiler.getSourceManager(),
+                        compiler.getLangOpts());
+                if (macroName == "SAL_EXCEPTION_DLLPUBLIC_EXPORT")
+                    return true;
+                if (macroName == "VCL_PLUGIN_PUBLIC")
+                    return true;
+                macroLoc = compiler.getSourceManager().getImmediateMacroCallerLoc(macroLoc);
+            }
+            for (auto it = p->method_begin(); it != p->method_end(); ++it) {
+                if (!it->hasInlineBody()) {
+                    return true;
+                }
+            }
+            report(
+                DiagnosticsEngine::Warning,
+                "unnecessary *DLLPUBLIC declaration for class completely defined in header file",
+                a->getLocation())
+                << decl->getSourceRange();
         }
     }
-};
-
-static loplugin::Plugin::Registration<Visitor> reg("dllprivate");
-
+    return true;
 }
 
+static loplugin::Plugin::Registration<DllMacro> reg("dllmacro");
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */

connectivity/source/inc/RowFunctionParser.hxx

@@ -55,7 +55,7 @@ public:
 /** This exception is thrown, when the arithmetic expression
     parser failed to parse a string.
     */
-struct OOO_DLLPUBLIC_DBTOOLS ParseError
+struct ParseError
 {
     ParseError( const char* ) {}
 };

desktop/source/deployment/inc/dp_ucb.h

@@ -36,7 +36,7 @@ class Content;
 
 namespace dp_misc {
 
-struct DESKTOP_DEPLOYMENTMISC_DLLPUBLIC StrTitle
+struct StrTitle
 {
     static css::uno::Sequence< OUString > getTitleSequence()
     {

include/basegfx/color/bcolor.hxx

@@ -42,7 +42,7 @@ namespace basegfx
 
         @see B3DTuple
     */
-    class SAL_WARN_UNUSED BASEGFX_DLLPUBLIC BColor : public B3DTuple
+    class SAL_WARN_UNUSED BColor : public B3DTuple
     {
     public:
         /** Create a Color with red, green and blue components from [0.0 to 1.0]

include/basegfx/matrix/b2dhommatrixtools.hxx

@@ -148,7 +148,7 @@ namespace basegfx
 {
     namespace utils
     {
-        class BASEGFX_DLLPUBLIC B2DHomMatrixBufferedDecompose
+        class B2DHomMatrixBufferedDecompose
         {
         private:
             B2DVector              maScale;

include/basegfx/numeric/ftools.hxx

@@ -148,7 +148,7 @@ namespace basegfx
      */
     BASEGFX_DLLPUBLIC double snapToNearestMultiple(double v, const double fStep);
 
-    class BASEGFX_DLLPUBLIC fTools
+    class fTools
     {
     public:
         /// Get threshold value for equalZero and friends

include/basegfx/pixel/bpixel.hxx

@@ -27,7 +27,7 @@
 
 namespace basegfx
 {
-    class BASEGFX_DLLPUBLIC BPixel
+    class BPixel
     {
     protected:
         union

include/comphelper/propertycontainerhelper.hxx

@@ -32,7 +32,7 @@ namespace comphelper
 
 
 // infos about one single property
-struct COMPHELPER_DLLPUBLIC PropertyDescription
+struct PropertyDescription
 {
     // the possibilities where a property holding object may be located
     enum class LocationType

include/connectivity/FValue.hxx

@@ -472,7 +472,7 @@ namespace connectivity
 
 
     /// TSetBound is a functor to set the bound value with e.q. for_each call
-    struct OOO_DLLPUBLIC_DBTOOLS TSetBound
+    struct TSetBound
     {
         bool m_bBound;
         TSetBound(bool _bBound) : m_bBound(_bBound){}
@@ -482,7 +482,7 @@ namespace connectivity
 
 
     /// TSetBound is a functor to set the bound value with e.q. for_each call
-    struct OOO_DLLPUBLIC_DBTOOLS TSetRefBound
+    struct TSetRefBound
     {
         bool m_bBound;
         TSetRefBound(bool _bBound) : m_bBound(_bBound){}

include/filter/msfilter/dffrecordheader.hxx

@@ -26,7 +26,7 @@
 #include <tools/solar.h>
 #include <tools/stream.hxx>
 
-class MSFILTER_DLLPUBLIC DffRecordHeader
+class DffRecordHeader
 {
 public:
     sal_uInt8   nRecVer; // may be DFF_PSFLAG_CONTAINER

include/filter/msfilter/mscodec.hxx

@@ -476,13 +476,13 @@ struct MSFILTER_DLLPUBLIC EncryptionVerifierRC4
     EncryptionVerifierRC4();
 };
 
-struct MSFILTER_DLLPUBLIC StandardEncryptionInfo
+struct StandardEncryptionInfo
 {
     EncryptionStandardHeader header;
     EncryptionVerifierAES    verifier;
 };
 
-struct MSFILTER_DLLPUBLIC RC4EncryptionInfo
+struct RC4EncryptionInfo
 {
     EncryptionStandardHeader header;
     EncryptionVerifierRC4 verifier;

include/filter/msfilter/svdfppt.hxx

@@ -117,7 +117,7 @@ struct SdHyperlinkEntry
 };
 
 // Helper class for reading the PPT InteractiveInfoAtom
-struct MSFILTER_DLLPUBLIC PptInteractiveInfoAtom
+struct PptInteractiveInfoAtom
 {
     sal_uInt32          nSoundRef;
     sal_uInt32          nExHyperlinkId;

include/filter/msfilter/util.hxx

@@ -70,7 +70,7 @@ MSFILTER_DLLPUBLIC OString ConvertColor( const Color &rColor, bool bAutoColor =
 
 
 /** Paper size in 1/100 millimeters. */
-struct MSFILTER_DLLPUBLIC ApiPaperSize
+struct ApiPaperSize
 {
     sal_Int32           mnWidth;
     sal_Int32           mnHeight;

include/svx/transfrmhelper.hxx

@@ -24,7 +24,7 @@
 #include <tools/fldunit.hxx>
 #include <tools/mapunit.hxx>
 
-class SVX_DLLPUBLIC TransfrmHelper
+class TransfrmHelper
 {
 public:
     static void ConvertRect(basegfx::B2DRange& rRange, const sal_uInt16 nDigits, const MapUnit ePoolUnit, const FieldUnit eDlgUnit)

include/svx/xmlexchg.hxx

@@ -42,7 +42,7 @@ namespace svx
     //= OXFormsDescriptor
 
 
-    struct SVX_DLLPUBLIC OXFormsDescriptor {
+    struct OXFormsDescriptor {
 
         OUString szName;
         OUString szServiceName;

include/tools/svborder.hxx

@@ -23,7 +23,7 @@
 #include <tools/toolsdllapi.h>
 #include <tools/gen.hxx>
 
-class SAL_WARN_UNUSED TOOLS_DLLPUBLIC SvBorder
+class SAL_WARN_UNUSED SvBorder
 {
     long nTop, nRight, nBottom, nLeft;
 

include/vcl/bitmap.hxx

@@ -110,7 +110,7 @@ enum class BmpFilter
     Unknown = 65535
 };
 
-class VCL_DLLPUBLIC BmpFilterParam
+class BmpFilterParam
 {
 public:
 

include/vcl/commandevent.hxx

@@ -206,7 +206,7 @@ enum class ShowDialogId
     About             = 2,
 };
 
-class VCL_DLLPUBLIC CommandDialogData
+class CommandDialogData
 {
     ShowDialogId   m_nDialogId;
 public:
@@ -242,7 +242,7 @@ enum class MediaCommand
     NextTrackHold         = 21,// Button Right holding pressed.
 };
 
-class VCL_DLLPUBLIC CommandMediaData
+class CommandMediaData
 {
     MediaCommand m_nMediaId;
     bool m_bPassThroughToOS;
@@ -270,7 +270,7 @@ public:
     sal_uLong          GetEnd() const { return mnEnd; }
 };
 
-class VCL_DLLPUBLIC CommandSwipeData
+class CommandSwipeData
 {
     double mnVelocityX;
 public:
@@ -286,7 +286,7 @@ public:
 };
 
 
-class VCL_DLLPUBLIC CommandLongPressData
+class CommandLongPressData
 {
     double mnX;
     double mnY;

include/vcl/pointr.hxx

@@ -23,7 +23,7 @@
 #include <vcl/dllapi.h>
 #include <vcl/ptrstyle.hxx>
 
-class VCL_DLLPUBLIC Pointer
+class Pointer
 {
     PointerStyle    meStyle;
 

sc/inc/colorscale.hxx

@@ -91,7 +91,7 @@ enum ScAxisPosition
 
 }
 
-struct SC_DLLPUBLIC ScDataBarFormatData
+struct ScDataBarFormatData
 {
     ScDataBarFormatData():
         maAxisColor(COL_BLACK),

sw/inc/ndindex.hxx

@@ -129,7 +129,7 @@ inline std::ostream &operator <<(std::ostream& s, const SwNodeIndex& index)
 
 // SwRange
 
-class SW_DLLPUBLIC SwNodeRange
+class SwNodeRange
 {
 public:
     SwNodeIndex aStart;

sw/source/filter/inc/fltshell.hxx

@@ -47,7 +47,7 @@ inline bool SwFltGetFlag(sal_uLong nFieldFlags, int no)
 
 //Subvert the Node/Content system to get positions which don't update as
 //content is appended to them
-struct SW_DLLPUBLIC SwFltPosition
+struct SwFltPosition
 {
 public:
     SwNodeIndex m_nNode;

vcl/inc/headless/svpgdi.hxx

@@ -71,7 +71,7 @@ typedef void (*damageHandler)(void* handle,
                               sal_Int32 nExtentsX, sal_Int32 nExtentsY,
                               sal_Int32 nExtentsWidth, sal_Int32 nExtentsHeight);
 
-struct VCL_DLLPUBLIC DamageHandler
+struct DamageHandler
 {
     void *handle;
     damageHandler damaged;