build: Fix warnings about control paths

[HuidaeCho]

This PR fixes control path warnings by adding a return statement after fatal. For noreturn functions, use void. See https://gcc.gnu.org/onlinedocs/gcc-3.4.3/gcc/Function-Attributes.html.

It does not make sense for a noreturn function to have a return type other than void.

[echoix]

I also found the current GCC docs (15.1.0) for the equivalent contents linked from the GCC 3.4.3 docs.

https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute

https://gcc.gnu.org/onlinedocs/gcc-15.1.0/gcc/Function-Attributes.html

I had one question, where I'm not sure, otherwise it is straightforward.

[echoix]

I know this is under a G_fatal_error, so shouldn't be reached. But the 4 usages of this function in this file only check with if (is_flag(p)) {}, so a non-zero value, here -1, doesn't mean the same.

[nilason]

Code after G_fatal_error is never reached and thus unnecessary. (Competent static analysers recognises this too).

[HuidaeCho]

I know this is under a G_fatal_error, so shouldn't be reached. But the 4 usages of this function in this file only check with if (is_flag(p)) {}, so a non-zero value, here -1, doesn't mean the same.

You're right. We could do if (is_flag(p) == 1), but again, we'll never reach return -1 anyway. It's just there to suppress the warning.

[nilason]

This is also never reached. I wonder how you triggered these warnings?

[HuidaeCho]

I think MSVC reports these warnings by default:

C:\Users\hcho\usr\grass\grass\raster\r.li\r.li.daemon\avl.c(230,1): warning C4715: 'avl_individua': not all control paths return a value [C:\Users\hcho\usr\grass\grass\build27\raster\r.li\grass_rli.vcxproj]

[HuidaeCho]

https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-1-c4715?view=msvc-170

[echoix]

At the end of that page, it points ta a link that shows how to use __declspec(noreturn) to annotate a function that never returns, for that analysis.

https://learn.microsoft.com/en-us/cpp/cpp/noreturn?view=msvc-170

[HuidaeCho]

That's for void functions technically. Or

It does not make sense for a noreturn function to have a return type other than void.

https://gcc.gnu.org/onlinedocs/gcc-3.4.3/gcc/Function-Attributes.html

[nilason]

G_fatal_error is attributed with __attribute__((noreturn)):

grass/include/grass/defs/gis.h

Lines 312 to 313 in d129608

	void G_fatal_error(const char *, ...) __attribute__((format(printf, 1, 2)))
	__attribute__((noreturn));

maybe that needs an update for windows?

This is a very common practice (no return after G_fatal_error) in GRASS code, so this should be solved in a more general way.

Something like:

void G_fatal_error(const char *, ...) __attribute__((format(printf, 1, 2))) 
#if defined(_MSC_VER)
__declspec(noreturn);
#else
 __attribute__((noreturn));
#endif

[nilason]

If my proposed change works, I'd suggest defining a macro in gis.h:

#if defined(_MSC_VER)
#define G_NORETURN __declspec(noreturn)
#else
#define G_NORETURN __attribute__((noreturn))
#endif

which then can be added:

void G_fatal_error(const char *, ...) __attribute__((format(printf, 1, 2))) G_NORETURN;

[nilason]

Or using the C11 keyword _Noreturn

#if  __STDC_VERSION__ < 202311L
#define G_NORETURN _Noreturn
#else
#define G_NORETURN [[noreturn]]
#endif

...

G_NORETURN void G_fatal_error(const char *, ...) __attribute__((format(printf, 1, 2)));

(untested)

[nilason]

The _Noreturn suggestion above doesn't work well with C++, however this seems to work:

#if __STDC_VERSION__ < 202311L
#if defined(_MSC_VER)
#define G_NORETURN __declspec(noreturn)
#else
#define G_NORETURN __attribute__((noreturn))
#endif
#else
#define G_NORETURN [[noreturn]]
#endif

(The [[noreturn]] attribute is added to C23 as well as C++11).

[wenzeslaus]

G_NORETURN seems reasonable to me although for the future plain [[noreturn]] would be preferable.