enabled and mitigated -Wdocumentation and -Wdocumentation-unknown-command Clang compiler warnings

[firewave]

No description provided.

[firewave]

I removed the empty fields as there's no point in having them if they have no content.

To properly detect the incomplete documentation of functions there's a Doxygen option you can set to bail out on those. That will not fail on completely undocumented functions. I need to look this up again as it has been a while.

[matt335672]

Possibly needs a [-Werror] adding as-per my comment on #2829

[firewave]

Possibly needs a [-Werror] adding as-per my comment on #2829

Done.

[firewave]

In file included from mkfv1.c:26:
In file included from /usr/include/freetype2/freetype/freetype.h:24:
In file included from /usr/include/freetype2/freetype/config/ftconfig.h:46:
/usr/include/freetype2/freetype/config/integer-types.h:85:6: error: unknown command tag name [-Werror,-Wdocumentation-unknown-command]
   * @type:
     ^~~~~

Even though I adjusted the compiler flag for the include folder it is still being treated as a non-system include. It could just be an issue within Clang though.

[matt335672]

Interesting.

You can use make V=1 to get the actual commands output by make to be echoed. That might give you a clue.

[firewave]

Interesting.

You can use make V=1 to get the actual commands output by make to be echoed. That might give you a clue.

Thanks. That helped a lot:

-I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4

These should obviously all be -isystem instead. That is the result from a pkg-config call:

FREETYPE2_CFLAGS='-I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -pthread '

$pkg-config --cflags freetype2
-I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/harfbuzz -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -pthread

Interestingly all packages seem to use -I. I am kind of surprised that it doesn't seem to cause havoc with compilers and static analysis - that might indicate that people do not care a lot about those or no longer use pkg-config. But let's stay on topic...

[matt335672]

I've had a quick look at this.

The problem seems to be that stuff referenced off /usr/include is fine by default. However the freetype includes aren't - as you have seen, they are in other directories. So this is a freetype2 problem only.

You could try something like this:-

--- a/configure.ac
+++ b/configure.ac
@@ -205,6 +205,9 @@ AX_GCC_FUNC_ATTRIBUTE([format])
 AX_TYPE_SOCKLEN_T
 AX_CFLAGS_WARN_ALL
 AX_APPEND_COMPILE_FLAGS([-Wwrite-strings])
+AX_APPEND_COMPILE_FLAGS([-Wdocumentation -Wdocumentation-unknown-command], ,[-Werror])
+# Can we use -isystem to specify includes?
+AX_CHECK_COMPILE_FLAG([-isystem.], [ISYSTEM_IS_SUPPORTED=yes])
 
 AM_COND_IF([LINUX],
   [AX_APPEND_COMPILE_FLAGS([-Werror])]) # bsd has warnings that have not been fixed yet
@@ -295,6 +298,10 @@ fi
 # freetype2 release. See docs/VERSIONS.TXT in the freetype2
 # source for a table of correspondences. If you change one
 # of the below defines, change both.
+#
+# Freetype includes are not on the usual system include path. Check
+# that -isystem is being used rather than -I, or features such as
+# clang's -Wdocumentation get broken.
 m4_define([FT2_REQUIRED_VERSION], [2_8_0])
 m4_define([FT2_REQUIRED_MODVERSION], [20.0.14])
 case "$with_freetype2" in
@@ -305,6 +312,9 @@ case "$with_freetype2" in
         PKG_CHECK_MODULES([FREETYPE2], [freetype2 >= FT2_REQUIRED_MODVERSION],
             [use_freetype2=yes],
             [AC_MSG_ERROR([please install version FT2_REQUIRED_VERSION or later of libfreetype6-dev or freetype-devel])])
+        if test x$ISYSTEM_IS_SUPPORTED = xyes; then
+            FREETYPE2_CFLAGS="`echo $FREETYPE2_CFLAGS | sed -e 's/-I/-isystem/g'`"
+        fi
         ;;
     /*) AC_MSG_CHECKING([for freetype2 in $with_freetype2])
         if test -d $with_freetype2/lib; then
@@ -317,7 +327,11 @@ case "$with_freetype2" in
         fi
 
         if test -f $with_freetype2/include/freetype2/ft2build.h; then
-            FREETYPE2_CFLAGS="-I $with_freetype2/include/freetype2"
+            if test x$ISYSTEM_IS_SUPPORTED = xyes; then
+                FREETYPE2_CFLAGS="-isystem $with_freetype2/include/freetype2"
+            else
+                FREETYPE2_CFLAGS="-I $with_freetype2/include/freetype2"
+            fi
         else
             AC_MSG_RESULT([no])
             AC_MSG_ERROR([Can't find $with_freetype2/include/freetype2/ft2build.h])

That should work with compilers that don't support -isystem

[firewave]

I guess we should not mess with the way includes are handled. Maybe the warning should be suppressed just in code.

[firewave]

I am having issues with the updated submodules locally. Those always irked me and also my IDE also doesn't handle them automatically so I need to figure out how to get the updated hashes working.

[firewave]

I am having issues with the updated submodules locally.

Resolved and rebased - let's see where we are at now...