Update comment to allow GCC skip fallthrough warnings

[seisman]

GCC 7 reports a lot of -Wimplicit-fallthrough warnings. See https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html and
https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/ for explanations.

For GCC, setting -Wimplicit-fallthrough=2 can let GCC understand the magic /* fall through */ comments and skip these warnings. However, Apple Clang only supports -Wimplicit-fallthrough. Any thoughts?

[PaulWessel]

Well, I guess it makes sense to add the /* fall through */ comment for our own sake as well (we do have some comments about this in places but did not know about the magic phrase), and perhaps clang eventually will adopt the same solution.

[joa-quim]

WE will be triggering a couple of them there are there on purpose and where already reported by coverity.

[seisman]

According to the GCC manual, -Wextra enables -Wimplicit-fallthrough, which is also equivalent to -Wimplicit-fallthrough=3.

When -Wimplicit-fallthrough=3 is set, comments match one of the following regex are accepted:

-fallthrough
@fallthrough@
lint -fallthrough[ \t]*
[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*(Else,? |Intentional(ly)? )?Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?
[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?

The regex looks really complicated, but the following comment is good:

/* Intentionally fall through - anything we want to say */

Maybe we should change the "fall-through" comments to this one?

[PaulWessel]

Yes, that is probably safe. So that second part is specific to why we fall through in each instance?

[seisman]

So that second part is specific to why we fall through in each instance?

Yes, and the - is required.

[seisman]

I've updated most comments to match the fall-through regex, and the number of fall-through warnings decreases.

We still have some warnings, mostly in gmt_init.c. For example, GCC reports fall-through warning for following snippet, because there is no "break" statement in the if-clause.

case GMTCASE_INPUT_CLOCK_FORMAT:
	if (gmt_M_compat_check (GMT, 4))	/* GMT4: */
		GMT_COMPAT_WARN;
	else { error = gmtinit_badvalreport (GMT, keyword); break; }	/* Not recognized so give error message */
case GMTCASE_FORMAT_CLOCK_IN:
	strncpy (value, GMT->current.setting.format_clock_in,  GMT_BUFSIZ-1);
	break;

I tried to add a comment line in the if-clause, but it didn't work. It seems I have to add the comment line exactly before the next case.

case GMTCASE_INPUT_CLOCK_FORMAT:
	if (gmt_M_compat_check (GMT, 4))	/* GMT4: */
		GMT_COMPAT_WARN;
	else { error = gmtinit_badvalreport (GMT, keyword); break; }	/* Not recognized so give error message */
        /* Intentionally fall through */
case GMTCASE_FORMAT_CLOCK_IN:
	strncpy (value, GMT->current.setting.format_clock_in,  GMT_BUFSIZ-1);
	break;

It's not ideal, but I don't see a workaround.

[PaulWessel]

OK< so your last example works? I think that is pretty good.

[seisman]

@PaulWessel Now GCC doesn't report any fall-through warnings.

[PaulWessel]

Fantastic, thanks!