[clang][Sema] Avoid duplicate diagnostics for unreachable fallthrough attribute

[hazohelet]

Summary

This patch checks whether -Wunreachable-code-fallthrough is enabled when clang encounters unreachable fallthrough attributes and, if so, suppresses code will never be executed warning to avoid duplicate warnings.
This fixes https://github.com/llvm/llvm-project/issues/60416

[shafik]

clang/lib/Analysis/ReachableCode.cpp

675

Your passing through DiagnosticsEngine just to check if diag::warn_unreachable_fallthrough_attr is ignored. I think it would be better just to pass through a boolean flag. This would avoid one include as well.

[hazohelet]

Address comments from @shafik

• Pass bool flag UnreachableFallThroughDiagIsEnabled instead of DiagnosticsEngine

[aaron.ballman]

clang/include/clang/Analysis/Analyses/ReachableCode.h

65

We don't typically use const on value types in the project, so dropping this for style consistency. You should make the same edit elsewhere in the patch as well.

clang/lib/Analysis/ReachableCode.cpp

680

Would it be cleaner (less changes) to pass a boolean into this callback to say "the statement had a fallthrough attribute on it" so that UnreachableCodeHandler::HandleUnreachable() can check to see whether the unreachable fallthrough diagnostic is enabled or not? It has access to a Sema object, so it should be able to do the work. FindUnreachableCode() would then not have to change its signature and it keeps the "should I suppress this diagnostic logic" nearby where we issue the diagnostic along with the other duplicate handling logic.

[hazohelet]

Address comments from @aaron.ballman

• Dropped const qualifier from value objects for style consistency
• Passed boolean HasFallThroughAttr to callback and handled warning suppression in HandleUnreachable in order to make code cleaner.

[aaron.ballman]

Thank you, I think this is heading in the right direction! Can you also add a release note to clang/docs/ReleaseNotes.rst about the fix?

clang/lib/Analysis/ReachableCode.cpp

636–638

NFC, just a stylistic choice.

Alternatively: change const AttributedStmt *AS = ... into const auto *AS = ...

722

Accidental whitespace change?

[hazohelet]

Address comments from @aaron.ballman

• NFC stylistic changes

Thanks for the review! Since I don't have commit access, would you please land this patch for me?
Please use "Takuya Shimizu <[e-mail]>" for the patch attribution.

[aaron.ballman]

LGTM!