enkisoftware / imgui_markdown

Markdown for Dear ImGui
zlib License
1.11k stars 78 forks source link

Implicit fallthrough warning/error #26

Open howprice opened 7 months ago

howprice commented 7 months ago

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
dougbinks commented 6 months ago

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.

howprice commented 6 months ago

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

clshortfuse commented 3 months ago
      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/