denik added inline comments.
================ Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:155 + break; + } +} ---------------- vaibhav.y wrote: > Does this need an `llvm_unreachable` after the switch? I guess no, because of `break`s. But... Although `case DiagnosticsEngine::Ignored` unreachable was copied from TextDiagnostic it looks a bit confusing to me. Do we want to store `Ignored` with `SarifResultLevel::None` or keep it unreachable? With the latter I would replace `case DiagnosticsEngine::Ignored` with `default` to cover new unhandled types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131632/new/ https://reviews.llvm.org/D131632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
