pirama added inline comments.
================
Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:111
+void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC,
+SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags);
+
----------------
Fix indentation.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2007
+static void flushDiagnostics(Sema &S,
+SmallVector<clang::sema::PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags)
{
+ for (const auto &D : PossiblyUnreachableDiags)
----------------
Should `PossiblyUnreachableDiags` be const?
Also, fix indentation.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2015
+
+ if (!PossiblyUnreachableDiags.empty()) {
+ bool analyzed = false;
----------------
How about returning early if `PossiblyUnreachableDiags` here is empty? That'll
avoid putting the entire logic of this function in the `true` branch.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2031
+ CFGReverseBlockReachabilityAnalysis *cra =
+ AC.getCFGReachablityAnalysis();
+ // FIXME: We should be able to assert that block is non-null, but
----------------
`getCFGReachablityAnalysis` has a typo. It's missing an 'i'. Consider fixing
this in a separate patch.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2051
+
+ if (!analyzed)
+ flushDiagnostics(S, PossiblyUnreachableDiags);
----------------
Rewrite this as the `else` clause for `if (AC.getCFG())`? In the current
structure, it's not immediately clear that `flushDiagnostics` is called iff
`AC.getCFG()` fails to return a valid CFG.
Upon further reading, this seems to be refactored from another function below
so it probably makes sense to leave it as-is.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63889/new/
https://reviews.llvm.org/D63889
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
