Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add [[fallthrough]] attribute to case statements when appropriate. #1314

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

Cory-Kramer
Copy link
Contributor

I added [[fallthrough]] attributes to case statements when fallthrough behavior was intended.
Fixes build warnings -Wimplicit-fallthrough.

opensubdiv/opensubdiv/bfr/hash.cpp:392:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  392 |     case 14:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:392:5: note: insert '[[fallthrough]];' to silence this warning
  392 |     case 14:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:392:5: note: insert 'break;' to avoid fall-through
  392 |     case 14:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:395:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  395 |     case 13:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:395:5: note: insert '[[fallthrough]];' to silence this warning
  395 |     case 13:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:395:5: note: insert 'break;' to avoid fall-through
  395 |     case 13:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:398:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  398 |     case 12:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:398:5: note: insert '[[fallthrough]];' to silence this warning
  398 |     case 12:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:398:5: note: insert 'break;' to avoid fall-through
  398 |     case 12:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:405:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  405 |     case 10:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:405:5: note: insert '[[fallthrough]];' to silence this warning
  405 |     case 10:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:405:5: note: insert 'break;' to avoid fall-through
  405 |     case 10:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:408:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  408 |     case 9:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:408:5: note: insert '[[fallthrough]];' to silence this warning
  408 |     case 9:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:408:5: note: insert 'break;' to avoid fall-through
  408 |     case 9:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:411:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  411 |     case 8:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:411:5: note: insert '[[fallthrough]];' to silence this warning
  411 |     case 8:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:411:5: note: insert 'break;' to avoid fall-through
  411 |     case 8:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:417:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  417 |     case 6:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:417:5: note: insert '[[fallthrough]];' to silence this warning
  417 |     case 6:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:417:5: note: insert 'break;' to avoid fall-through
  417 |     case 6:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:420:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  420 |     case 5:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:420:5: note: insert '[[fallthrough]];' to silence this warning
  420 |     case 5:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:420:5: note: insert 'break;' to avoid fall-through
  420 |     case 5:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:423:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  423 |     case 4:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:423:5: note: insert '[[fallthrough]];' to silence this warning
  423 |     case 4:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:423:5: note: insert 'break;' to avoid fall-through
  423 |     case 4:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:429:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  429 |     case 2:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:429:5: note: insert '[[fallthrough]];' to silence this warning
  429 |     case 2:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:429:5: note: insert 'break;' to avoid fall-through
  429 |     case 2:
      |     ^
      |     break; 
opensubdiv/opensubdiv/bfr/hash.cpp:432:5: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
  432 |     case 1:
      |     ^
opensubdiv/opensubdiv/bfr/hash.cpp:432:5: note: insert '[[fallthrough]];' to silence this warning
  432 |     case 1:
      |     ^
      |     [[fallthrough]]; 
opensubdiv/opensubdiv/bfr/hash.cpp:432:5: note: insert 'break;' to avoid fall-through
  432 |     case 1:
      |     ^
      |     break; 
11 errors generated.

@davidgyu
Copy link
Member

Filed as internal issue #OSD-429

@barfowl
Copy link
Collaborator

barfowl commented Aug 1, 2023

My understanding is that [[fallthrough]] was introduced in C++17. At this point, OpenSubdiv does not use any features specific to C++17, so this change will make C++17 a new minimum requirement to build it.

@davidgyu, the USD repository contains similar code for its internal hashing. In the spirit of keeping Pixar's projects reasonably aligned, has the issue of how/when to suppress similar fall-through warnings been raised there?

@davidgyu
Copy link
Member

davidgyu commented Aug 3, 2023

That's right, we're not ready to make changes to the code that would require C++17.

This hasn't been addressed yet in the OpenUSD code base. There is work in progress to align with recent vfx platform versions which specify C++17 and when that work is complete we will consider incorporating C++17 specific features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants