18

I am trying to compile mitk on ubuntu and I got this error :

error: this statement may fall through [-Werror=implicit-fallthrough=]

Here there is a part of code :

      /** Get memory offset for a given image index */
      unsigned int GetOffset(const IndexType & idx) const
      {
       const unsigned int * imageDims = m_ImageDataItem->m_Dimensions;

        unsigned int offset = 0;
        switch(VDimension)
        {
        case 4:
         offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
        case 3:
        offset = offset + idx[2]*imageDims[0]*imageDims[1];
        case 2:
        offset  = offset + idx[0] + idx[1]*imageDims[0];
         break;
        }

        return offset;
      }

I would be appreciate for any help please.

vik_78
  • 1,107
  • 2
  • 13
  • 20
agasim
  • 183
  • 1
  • 1
  • 4
  • 1
    That fallthrough looks deliberate to me. – molbdnilo Apr 15 '19 at 15:59
  • 4
    Reopened. This does not look like a duplicate of the other question; the fall-through here looks deliberate, and the problem is the overly-aggressive compiler settings that turn fall-through into an error. – Pete Becker Apr 15 '19 at 16:00
  • 2
    If deliberate, add `[[fallthrough]]`. – Jesper Juhl Apr 15 '19 at 16:01
  • 1
    It looks like you're compiling with some setting that turns every warning into an error. If you do that you have to live with the consequences: the compiler thinks you're not smart enough to have intended to do what you did, so it refuses to compile your code. – Pete Becker Apr 15 '19 at 16:01
  • 1
    @PeteBecker I don't think it is deliberate at all. I think it is a misunderstanding that case 4 wont fall through to case 3 since 4 comes after 3, numerically. – NathanOliver Apr 15 '19 at 16:02
  • 1
    @PeteBecker your comment can be read as compiling with warnings as errors would be something bad. Unfortunately sometimes warning have false positives and in such case you only want to silence the compiler, not fix anything on the code – 463035818_is_not_an_ai Apr 15 '19 at 16:03
  • 1
    @user463035818 -- you got my message. Seriously: I know that some people like this setting. It's a royal pain when you're trying to write portable code, and have to dodge every compiler's peculiarities. An infinite loop with a conditional return statement can produce a warning that the code is missing a return statement (after the loop); adding a return statement produces a warning from a different compiler about unreachable code. – Pete Becker Apr 15 '19 at 16:03
  • 1
    @NathanOliver -- this looks like a cumulative calculation where the high-order parts are only used for high-dimensional objects. – Pete Becker Apr 15 '19 at 16:04
  • can you please clarify in the question if the fallthrough is intentional? ie for case 2, `offset` should be the sum of all terms? In your question it is actually not 100% clear if the warning is a false positive or not. – 463035818_is_not_an_ai Apr 15 '19 at 16:06

5 Answers5

15

Switch case statements will fall through by default. In the case of the shown program, if VDimension is 4 then all of

offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
offset = offset + idx[2]*imageDims[0]*imageDims[1];
offset  = offset + idx[0] + idx[1]*imageDims[0];

will be executed.

In some other languages, such as Pascal, only one case is executed and there is no concept of fall through. As such, programmers new to C++ may write fall through switches unintentionally.

If the fall through is unintentional, you need to add a break between each case to not fall through.

this statement may fall through

This warning notifies the programmer about the fall through. This warning option can be controlled with the GCC compiler switch -Wimplicit-fallthrough. It is not enabled by default and is not enabled by -Wall, but it is enabled by -Wextra.

Warnings become errors if the -Werror switch is used. -Werror is not enabled by default.

C++17 introduced [[fallthrough]] attribute, which can be used to explicitly document the fall through when it is intentional. The compiler should not warn if it is used:

        switch(VDimension)
        {
        case 4:
         offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
         [[fallthrough]];
        case 3:
         offset = offset + idx[2]*imageDims[0]*imageDims[1];
         [[fallthrough]];
        case 2:
         offset = offset + idx[0] + idx[1]*imageDims[0];
         break;
        }

Prior to C++17, GCC provides a language extension attribute __attribute__ ((fallthrough)) for the same purpose.

The fall through can also be documented with a comment, and Wimplicit-fallthrough may detect such comment depending on what value was used with the switch. More details in the documentation of GCC.

parsley72
  • 8,449
  • 8
  • 65
  • 98
eerorika
  • 232,697
  • 12
  • 197
  • 326
7

You should add keyword break to each case statement, if you don't do that, the code will run from case which it matches condition and continue to meet the

break;

for example: If VDimension = 4, then the code will run from case 4 => continue to case 3 => contine to case 2 then break. It means that it will execute below commands:

offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
offset = offset + idx[2]*imageDims[0]*imageDims[1];
offset  = offset + idx[0] + idx[1]*imageDims[0];
break;
return offset;

I think your code should be:

/** Get memory offset for a given image index */
  unsigned int GetOffset(const IndexType & idx) const
  {
   const unsigned int * imageDims = m_ImageDataItem->m_Dimensions;

    unsigned int offset = 0;
    switch(VDimension)
    {
    case 4:
     offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
     break;
    case 3:
     offset = offset + idx[2]*imageDims[0]*imageDims[1];
     break;
    case 2:
     offset  = offset + idx[0] + idx[1]*imageDims[0];
     break;
    }

    return offset;
  }
the boy
  • 235
  • 1
  • 6
  • 6
    Yes, that would get rid of the warning. But it would probably break the code, which looks like it intentionally relies on fallthrough. – Pete Becker Apr 15 '19 at 17:35
  • 1
    I don't think that is the case, but if it is, we need to remove some flag when compiling – the boy Apr 15 '19 at 23:48
3

If you're absolutely sure the warnings are inconsequential, remove the -Werror flag during compilation.

For projects which put the flag during configure stage, you can do

./configure --disable-werror

before running

make

Fortune
  • 162
  • 1
  • 4
1

Assuming the code is correct, just add -Wno-implicit-fallthrough to the compiler flags if you can. This is often CFLAGS or CXXFLAGS in the Makefile.

As others have said, the code for cases 4, 3, and 2 will be executed in that order when VDimension is 4 because there are no break statements to stop it. If that's the intended behavior (and I suspect it is) then the compiler is annoyingly warning you that the switch statement is doing exactly what it should do.

Raptor007
  • 366
  • 3
  • 10
0
      /** Get memory offset for a given image index */
  unsigned int GetOffset(const IndexType & idx) const
  {
   const unsigned int * imageDims = m_ImageDataItem->m_Dimensions;

    unsigned int offset = 0;
    switch(VDimension)
    {
    case 4:
     offset = offset + idx[3]*imageDims[0]*imageDims[1]*imageDims[2];
     break;
    case 3:
    offset = offset + idx[2]*imageDims[0]*imageDims[1];
     break;
    case 2:
    offset  = offset + idx[0] + idx[1]*imageDims[0];
     break;
    }

    return offset;
  }

Just add "break;" after finish any case.