aaron.ballman added inline comments.
================ Comment at: clang/include/clang/Analysis/Analyses/ReachableCode.h:65 + Callback &CB, + const bool UnreachableFallThroughDiagIsEnabled); }} // end namespace clang::reachable_code ---------------- 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. ================ Comment at: clang/lib/Analysis/ReachableCode.cpp:680 SourceLocation Loc = GetUnreachableLoc(S, R1, R2); CB.HandleUnreachable(UK, Loc, SilenceableCondVal, R1, R2); } ---------------- 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145842/new/ https://reviews.llvm.org/D145842 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
