Use [[nodiscard]] in SAL_WARN_UNUSED_RESULT where available

...which required some lax placements of SAL_WARN_UNUSED_RESULT to be fixed.
Also, Clang unfortunately is rather picky about the relative order of
SAL_WARN_UNUSED_RESULT expanding to [[nodiscard]] and uses of the DLLPUBLIC
macros (expanding to __attribute__(...) resp. __declspec(..) for clang-cl).

Change-Id: Iae6ca36bef97f1864873aefdb5f05c7f5e045ad3
Reviewed-on: https://gerrit.libreoffice.org/60274
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

15 files changed, 59 insertions, 28 deletions

diff --git a/config_host/config_global.h.in b/config_host/config_global.h.in
index d40630ab99c3..7913460d6bf8 100644
--- a/config_host/config_global.h.in
+++ b/config_host/config_global.h.in
@@ -23,6 +23,9 @@ Any change in this header will cause a rebuild of almost everything.
 /* Compiler supports __attribute__((warn_unused)). */
 #define HAVE_GCC_ATTRIBUTE_WARN_UNUSED 0
 
+/* [[nodiscard]] (C++17), __has_cpp_attribute(nodiscard) (C++2a): */
+#define HAVE_CPP_ATTRIBUTE_NODISCARD 0
+
 /* Guaranteed copy elision (C++17), __cpp_guaranteed_copy_elision (C++2a): */
 #define HAVE_CPP_GUARANTEED_COPY_ELISION 0
 
diff --git a/configure.ac b/configure.ac
index 9861188660e2..56b798644f9f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -6408,6 +6408,33 @@ if test "$GCC" = yes; then
 fi
 AC_SUBST([HAVE_GCC_FNO_SIZED_DEALLOCATION])
 
+AC_MSG_CHECKING([[whether $CXX supports [[nodiscard]]]])
+AC_LANG_PUSH([C++])
+save_CXXFLAGS=$CXXFLAGS
+CXXFLAGS="$CXXFLAGS $CXXFLAGS_CXX11"
+dnl Unknown attributes must be ignored by compilers, but they do emit warnings about them:
+if test "$_os" = WINNT; then
+    CXXFLAGS="$CXXFLAGS /WX"
+else
+    CXXFLAGS="$CXXFLAGS -Werror"
+fi
+AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+        // There appears to be no feature-test macro for __has_cpp_attribute in C++2a, approximate
+        // by checking for __cplusplus:
+        #if __cplusplus > 201703L
+        #if !__has_cpp_attribute(nodiscard)
+        #error
+        #endif
+        #else
+        [[nodiscard]] int f();
+        #endif
+    ]])], [
+        AC_DEFINE([HAVE_CPP_ATTRIBUTE_NODISCARD],[1])
+        AC_MSG_RESULT([yes])
+    ], [AC_MSG_RESULT([no])])
+CXXFLAGS=$save_CXXFLAGS
+AC_LANG_POP([C++])
+
 AC_MSG_CHECKING([whether $CXX supports guaranteed copy elision])
 AC_LANG_PUSH([C++])
 save_CXXFLAGS=$CXXFLAGS
diff --git a/include/com/sun/star/uno/Reference.h b/include/com/sun/star/uno/Reference.h
index a7b62926a0c7..a44831b6c360 100644
--- a/include/com/sun/star/uno/Reference.h
+++ b/include/com/sun/star/uno/Reference.h
@@ -571,13 +571,13 @@ public:
         @param rRef interface reference
         @return interface reference of demanded type (may be null)
     */
-    inline static SAL_WARN_UNUSED_RESULT Reference< interface_type > SAL_CALL query( const BaseReference & rRef );
+    SAL_WARN_UNUSED_RESULT inline static Reference< interface_type > SAL_CALL query( const BaseReference & rRef );
     /** Queries given interface for type interface_type.
 
         @param pInterface interface pointer
         @return interface reference of demanded type (may be null)
     */
-    inline static SAL_WARN_UNUSED_RESULT Reference< interface_type > SAL_CALL query( XInterface * pInterface );
+    SAL_WARN_UNUSED_RESULT inline static Reference< interface_type > SAL_CALL query( XInterface * pInterface );
 };
 
 }
diff --git a/include/filter/msfilter/msdffimp.hxx b/include/filter/msfilter/msdffimp.hxx
index 6bfa44087b51..f6d0b76d5a20 100644
--- a/include/filter/msfilter/msdffimp.hxx
+++ b/include/filter/msfilter/msdffimp.hxx
@@ -574,11 +574,11 @@ public:
                                            sal_uInt32 nMaxLen,
                                            bool bUniCode);
 
-    static bool ReadCommonRecordHeader( SvStream& rSt,
+    SAL_WARN_UNUSED_RESULT static bool ReadCommonRecordHeader( SvStream& rSt,
                                         sal_uInt8& rVer,
                                         sal_uInt16& rInst,
                                         sal_uInt16& rFbt,
-                                        sal_uInt32& rLength) SAL_WARN_UNUSED_RESULT;
+                                        sal_uInt32& rLength);
 
 // TODO: provide proper documentation here
     /** constructor
diff --git a/include/osl/file.h b/include/osl/file.h
index 038631a86618..133b93de0c93 100644
--- a/include/osl/file.h
+++ b/include/osl/file.h
@@ -717,8 +717,8 @@ SAL_DLLPUBLIC oslFileError SAL_CALL osl_openFile(
     @see    osl_openFile()
     @see    osl_getFilePos()
 */
-SAL_DLLPUBLIC oslFileError SAL_CALL osl_setFilePos(
-        oslFileHandle Handle, sal_uInt32 uHow, sal_Int64 uPos ) SAL_WARN_UNUSED_RESULT;
+SAL_WARN_UNUSED_RESULT SAL_DLLPUBLIC oslFileError SAL_CALL osl_setFilePos(
+        oslFileHandle Handle, sal_uInt32 uHow, sal_Int64 uPos );
 
 /** Retrieve the current position of the internal pointer of an open file.
 
diff --git a/include/osl/file.hxx b/include/osl/file.hxx
index a85f5fc7aeb0..a154aa4f27bc 100644
--- a/include/osl/file.hxx
+++ b/include/osl/file.hxx
@@ -1015,7 +1015,7 @@ public:
         @see getPos()
     */
 
-    RC setPos( sal_uInt32 uHow, sal_Int64 uPos ) SAL_WARN_UNUSED_RESULT
+    SAL_WARN_UNUSED_RESULT RC setPos( sal_uInt32 uHow, sal_Int64 uPos )
     {
         return static_cast< RC >( osl_setFilePos( _pData, uHow, uPos ) );
     }
diff --git a/include/rtl/stringconcat.hxx b/include/rtl/stringconcat.hxx
index cc3a9516b676..f1c1ca8b5e6e 100644
--- a/include/rtl/stringconcat.hxx
+++ b/include/rtl/stringconcat.hxx
@@ -221,8 +221,8 @@ struct ToStringHelper< OUStringConcat< T1, T2 > >
     };
 
 template< typename T1, typename T2 >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OStringConcat< T1, T2 >, ToStringHelper< T1 >::allowOStringConcat && ToStringHelper< T2 >::allowOStringConcat >::Type operator+( const T1& left, const T2& right )
     {
     return OStringConcat< T1, T2 >( left, right );
@@ -230,56 +230,56 @@ typename libreoffice_internal::Enable< OStringConcat< T1, T2 >, ToStringHelper<
 
 // char[N] and const char[N] need to be done explicitly, otherwise the compiler likes to treat them the same way for some reason
 template< typename T, int N >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OStringConcat< T, const char[ N ] >, ToStringHelper< T >::allowOStringConcat >::Type operator+( const T& left, const char (&right)[ N ] )
     {
     return OStringConcat< T, const char[ N ] >( left, right );
     }
 
 template< typename T, int N >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OStringConcat< const char[ N ], T >, ToStringHelper< T >::allowOStringConcat >::Type operator+( const char (&left)[ N ], const T& right )
     {
     return OStringConcat< const char[ N ], T >( left, right );
     }
 
 template< typename T, int N >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OStringConcat< T, char[ N ] >, ToStringHelper< T >::allowOStringConcat >::Type operator+( const T& left, char (&right)[ N ] )
     {
     return OStringConcat< T, char[ N ] >( left, right );
     }
 
 template< typename T, int N >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OStringConcat< char[ N ], T >, ToStringHelper< T >::allowOStringConcat >::Type operator+( char (&left)[ N ], const T& right )
     {
     return OStringConcat< char[ N ], T >( left, right );
     }
 
 template< typename T1, typename T2 >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OUStringConcat< T1, T2 >, ToStringHelper< T1 >::allowOUStringConcat && ToStringHelper< T2 >::allowOUStringConcat >::Type operator+( const T1& left, const T2& right )
     {
     return OUStringConcat< T1, T2 >( left, right );
     }
 
 template< typename T1, typename T2 >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OUStringConcat< T1, T2 >, ToStringHelper< T1 >::allowOUStringConcat && ToStringHelper< T2 >::allowOUStringConcat && libreoffice_internal::ConstCharArrayDetector< T1, void >::ok >::Type operator+( T1& left, const T2& right )
     {
     return OUStringConcat< T1, T2 >( left, right );
     }
 
 template< typename T1, typename T2 >
-inline
 SAL_WARN_UNUSED_RESULT
+inline
 typename libreoffice_internal::Enable< OUStringConcat< T1, T2 >, ToStringHelper< T1 >::allowOUStringConcat && ToStringHelper< T2 >::allowOUStringConcat && libreoffice_internal::ConstCharArrayDetector< T2, void >::ok >::Type operator+( const T1& left, T2& right )
     {
     return OUStringConcat< T1, T2 >( left, right );
diff --git a/include/sal/types.h b/include/sal/types.h
index c6c8b66d351b..875f121db1ae 100644
--- a/include/sal/types.h
+++ b/include/sal/types.h
@@ -292,7 +292,9 @@ typedef void *                   sal_Handle;
     Compilers that support a construct of this nature will emit a compile
     time warning on unchecked return value.
 */
-#if (defined __GNUC__ \
+#if defined __cplusplus && HAVE_CPP_ATTRIBUTE_NODISCARD
+#define SAL_WARN_UNUSED_RESULT [[nodiscard]]
+#elif (defined __GNUC__ \
      && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 1))) \
     || defined __clang__
 #   define SAL_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
diff --git a/include/svx/svdundo.hxx b/include/svx/svdundo.hxx
index 642b67954073..6d8896916bcd 100644
--- a/include/svx/svdundo.hxx
+++ b/include/svx/svdundo.hxx
@@ -129,7 +129,7 @@ protected:
 
     void ImpTakeDescriptionStr(const char* pStrCacheID, OUString& rStr, bool bRepeat = false) const;
 
-    static SAL_WARN_UNUSED_RESULT OUString GetDescriptionStringForObject( const SdrObject& _rForObject, const char* pStrCacheID, bool bRepeat = false );
+    SAL_WARN_UNUSED_RESULT static OUString GetDescriptionStringForObject( const SdrObject& _rForObject, const char* pStrCacheID, bool bRepeat = false );
 
     // #94278# new method for evtl. PageChange at UNDO/REDO
     void ImpShowPageOfThisObject();
diff --git a/include/svx/unoapi.hxx b/include/svx/unoapi.hxx
index 9c019a5a83a2..0f07186497fb 100644
--- a/include/svx/unoapi.hxx
+++ b/include/svx/unoapi.hxx
@@ -79,7 +79,7 @@ SVX_DLLPUBLIC bool SvxFieldUnitToMeasureUnit( const FieldUnit nVcl, short& eApi
  *
  * @throws std::exception
 */
-SVX_DLLPUBLIC SAL_WARN_UNUSED_RESULT OUString
+SAL_WARN_UNUSED_RESULT SVX_DLLPUBLIC OUString
     SvxUnogetApiNameForItem(const sal_uInt16 nWhich, const OUString& rInternalName);
 
 /**
@@ -88,7 +88,7 @@ SVX_DLLPUBLIC SAL_WARN_UNUSED_RESULT OUString
  *
  * @throws std::exception
 */
-SVX_DLLPUBLIC SAL_WARN_UNUSED_RESULT OUString
+SAL_WARN_UNUSED_RESULT SVX_DLLPUBLIC OUString
     SvxUnogetInternalNameForItem(const sal_uInt16 nWhich, const OUString& rApiName);
 
 #endif // INCLUDED_SVX_UNOAPI_HXX
diff --git a/include/tools/helpers.hxx b/include/tools/helpers.hxx
index 1e88e5725af4..2ce550c42e03 100644
--- a/include/tools/helpers.hxx
+++ b/include/tools/helpers.hxx
@@ -78,7 +78,7 @@ inline long FRound( double fVal )
 
 //valid range:  (-180,180]
 template <typename T>
-inline SAL_WARN_UNUSED_RESULT typename std::enable_if<std::is_signed<T>::value, T>::type
+SAL_WARN_UNUSED_RESULT inline typename std::enable_if<std::is_signed<T>::value, T>::type
 NormAngle180(T angle)
 {
     while (angle <= -180)
@@ -89,7 +89,7 @@ NormAngle180(T angle)
 }
 
 //valid range:  [0,360)
-template <typename T> inline SAL_WARN_UNUSED_RESULT T NormAngle360(T angle)
+template <typename T> SAL_WARN_UNUSED_RESULT inline T NormAngle360(T angle)
 {
     while (angle < 0)
         angle += 360;
diff --git a/include/tools/stream.hxx b/include/tools/stream.hxx
index 9b28c6a46f65..ce6db9ac4217 100644
--- a/include/tools/stream.hxx
+++ b/include/tools/stream.hxx
@@ -577,7 +577,7 @@ inline std::size_t write_uInt16_lenPrefixed_uInt8s_FromOUString(SvStream& rStrm,
     return write_uInt16_lenPrefixed_uInt8s_FromOString(rStrm, OUStringToOString(rStr, eEnc));
 }
 
-TOOLS_DLLPUBLIC bool checkSeek(SvStream &rSt, sal_uInt64 nOffset) SAL_WARN_UNUSED_RESULT;
+SAL_WARN_UNUSED_RESULT TOOLS_DLLPUBLIC bool checkSeek(SvStream &rSt, sal_uInt64 nOffset);
 
 // FileStream
 
diff --git a/include/vcl/vclptr.hxx b/include/vcl/vclptr.hxx
index 642a57ea5ec2..019f4c593c49 100644
--- a/include/vcl/vclptr.hxx
+++ b/include/vcl/vclptr.hxx
@@ -130,7 +130,7 @@ public:
      *
      * @tparam reference_type must be a subclass of vcl::Window
      */
-    template<typename... Arg> static SAL_WARN_UNUSED_RESULT VclPtr< reference_type > Create(Arg &&... arg)
+    template<typename... Arg> SAL_WARN_UNUSED_RESULT static VclPtr< reference_type > Create(Arg &&... arg)
     {
         return VclPtr< reference_type >( new reference_type(std::forward<Arg>(arg)...), SAL_NO_ACQUIRE );
     }
diff --git a/sc/inc/address.hxx b/sc/inc/address.hxx
index 9d123383c961..cc678752cdb2 100644
--- a/sc/inc/address.hxx
+++ b/sc/inc/address.hxx
@@ -340,7 +340,7 @@ public:
         @param  pDocument
                 The document for the maximum defined sheet number.
      */
-    SC_DLLPUBLIC SAL_WARN_UNUSED_RESULT bool Move( SCCOL nDeltaX, SCROW nDeltaY, SCTAB nDeltaZ,
+    SAL_WARN_UNUSED_RESULT SC_DLLPUBLIC bool Move( SCCOL nDeltaX, SCROW nDeltaY, SCTAB nDeltaZ,
             ScAddress& rErrorPos, const ScDocument* pDocument = nullptr );
 
     inline bool operator==( const ScAddress& rAddress ) const;
@@ -615,11 +615,11 @@ public:
         @param  pDocument
                 The document for the maximum defined sheet number.
      */
-    SC_DLLPUBLIC SAL_WARN_UNUSED_RESULT bool Move( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ,
+    SAL_WARN_UNUSED_RESULT SC_DLLPUBLIC bool Move( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ,
             ScRange& rErrorRange, const ScDocument* pDocument = nullptr );
 
     /** Same as Move() but with sticky end col/row anchors. */
-    SC_DLLPUBLIC SAL_WARN_UNUSED_RESULT bool MoveSticky( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ,
+    SAL_WARN_UNUSED_RESULT SC_DLLPUBLIC bool MoveSticky( SCCOL aDeltaX, SCROW aDeltaY, SCTAB aDeltaZ,
             ScRange& rErrorRange );
 
     SC_DLLPUBLIC void IncColIfNotLessThan(SCCOL nStartCol, SCCOL nOffset);
diff --git a/sw/source/filter/ww8/ww8scan.hxx b/sw/source/filter/ww8/ww8scan.hxx
index fefb240ed454..1579ccb83856 100644
--- a/sw/source/filter/ww8/ww8scan.hxx
+++ b/sw/source/filter/ww8/ww8scan.hxx
@@ -1863,8 +1863,7 @@ void SwapQuotesInField(OUString &rFormat);
 Word2CHPX ReadWord2Chpx(SvStream &rSt, std::size_t nOffset, sal_uInt8 nSize);
 std::vector<sal_uInt8> ChpxToSprms(const Word2CHPX &rChpx);
 
-bool checkRead(SvStream &rSt, void *pDest, sal_uInt32 nLength)
-    SAL_WARN_UNUSED_RESULT;
+SAL_WARN_UNUSED_RESULT bool checkRead(SvStream &rSt, void *pDest, sal_uInt32 nLength);
 
 //MS has a (slightly) inaccurate view of how many twips
 //are in the default letter size of a page