GNOME Bugzilla – Bug 741901
Clang cannot know that g_error don't return
Last modified: 2017-11-08 13:13:23 UTC
After -Werror=return-type was added to gnome-common a week ago, clang reports this type of error for a few modules: vinagre/vinagre-dirs.c:207:1: error: control may reach end of non-void function [-Werror,-Wreturn-type] In vinagre/vinagre-dirs.vala, the last line of the get_package_data_file function is error, which is translated to g_error, so it is not possible for this function to reach end and return an unknown value. I found the declaration of g_error in the preprocessed file,: static void g_error (const gchar *format, ...) __attribute__((analyzer_noreturn)); If I add _Noreturn to the declaration, the problem is fixed. I know we cannot simply do this in glib because C++ or older C compilers do not accept it. Is there any reason that clang should use the static function? Or we should modify the #ifdef check so it can use the macro instead?
This simple program can be used to reproduce the problem: #include <glib.h> #include <stdio.h> static const char* get_string (int c) { if (c > 1) { return "Good"; } g_error ("Sorry"); } int main (int argc, char *argv[]) { puts (get_string (argc)); return 0; } Error message: test.c:9:1: error: control may reach end of non-void function [-Werror,-Wreturn-type]
Created attachment 293536 [details] [review] gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use
Created attachment 293552 [details] [review] gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use Updated patch which causes less warnings
Does someone have time to review the patch?
Comment on attachment 293552 [details] [review] gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use Shouldn't we just use G_GNUC_NORETURN on g_error() since it's guaranteed to never return?
Yes, I think we can add G_GNUC_NORETURN, but the original problem is that G_ANALYZER_ANALYZING seems to be incorrect. Is there any reason that we should treat gcc and clang differently when the static analyzer is not used?
Created attachment 297121 [details] [review] gmessages: Add G_GNUC_NORETURN to g_error static function declaration This patch doesn't obsolete the previous patch. I still think that we should change G_ANALYZER_ANALYZING macro because either its name or its implementation is misleading.
Created attachment 300461 [details] [review] gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use Update the patch to work with current master branch.
These look good to me.
Attachment 300461 [details] pushed as d7a1d89 - gmacros: Only set G_ANALYZER_ANALYZING to 1 when clang static analyzer is in use Attachment 297121 [details] pushed as 93dadb1 - gmessages: Add G_GNUC_NORETURN to g_error static function declaration
*** Bug 734128 has been marked as a duplicate of this bug. ***