imgui_markdown icon indicating copy to clipboard operation
imgui_markdown copied to clipboard

Implicit fallthrough warning/error

Open howprice opened this issue 1 year ago • 3 comments

I'm not sure the best way to resolve this issue in the repo, but when compiling with GCC on Ubuntu with options -Wall -Wextra -pedantic -Wno-unknown-pragmas -Werror:

In file included from /some/folder/work/myproj/myproj/src/main.cpp:20:
/home/runner/work/iragui/iragui/libs/imgui_markdown/imgui_markdown.h: In function ‘void ImGui::Markdown(const char*, size_t, const ImGui::MarkdownConfig&)’:
/home/runner/work/iragui/iragui/libs/imgui_markdown/imgui_markdown.h:726:33: error: this statement may fall through [-Werror=implicit-fallthrough=]
  726 |                                 if( em.sym == c )
      |                                 ^~
/home/runner/work/myproj/myproj/libs/imgui_markdown/imgui_markdown.h:736:25: note: here
  736 |                         case Emphasis::RIGHT:
      |                         ^~~~
cc1plus: all warnings being treated as errors

This warning can be handled in C++17 with the [[fallthrough]] attribute (in fact the linked example is almost this very case!). However, prior to C++17 unknown attributes were not guaranteed to be ignored, so using this attribute could lead to warnings with other toolchains.

I guess one messy solution might be:

#if __cplusplus >= 201703L
    [[fallthrough]]
#endif

EDIT:

Actually this fix

case Emphasis::MIDDLE:
	if( em.sym == c )
	{
		em.state = Emphasis::RIGHT;
		em.text.stop = i;
		// pass through to case Emphasis::RIGHT
		[[fallthrough]]; 
	}
	 else
	{
		break;
	}
case Emphasis::RIGHT:

Results in MSVC warning:

warning C4468: the [[fallthrough]] attribute must be followed by a case label or a default label

howprice avatar Apr 29 '24 14:04 howprice

I'm not sure what to do about this one for now, especially given the issue with the MSVC warning.

I'll have a look at it when I get time, but for now I would disable the warning before including the header.

dougbinks avatar May 03 '24 14:05 dougbinks

Thanks. I've done that for now. Hopefully this wrinkle will flatten out over time.

howprice avatar May 03 '24 19:05 howprice

      case Emphasis::MIDDLE:
        if (em.sym != c) break;
        em.state = Emphasis::RIGHT;
        em.text.stop = i;
#if __cplusplus >= 201703L
      [[fallthrough]]
#endif
      case Emphasis::RIGHT:

This should work for Visual Studio 2017 and above: https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

clshortfuse avatar Aug 26 '24 16:08 clshortfuse