[PATCH] D127898: [clang][dataflow] Find unsafe locs after fixpoint

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:66
+  SourceLocations Locs;
+  for (const CFGBlock *Block : Context->getCFG()) {
+    // Skip blocks that were not evaluated.
----------------
xazax.hun wrote:
> While doing yet another iteration after we reached the fixed point is a valid approach, many analyses have a monotonic property when it comes to emitting diagnostics. In this case, when the analysis discovered that an optional access is unsafe in one of the iterations I would not expect it to become safe in subsequent iterations. In most of the cases this makes collecting diagnostics eagerly a viable option and saves us from doing one more traversal of the CFG. On the other hand, we need to be careful to not to emit duplicate diagnostics.
> 
> I wonder whether, from a user point of view, not having to do one more iteration is a more ergonomic interface. 
@xazax.hun Agreed, I don't think for this particular analysis there's a risk of diagnostics being prematurely added and then becoming invalid as the analysis progresses. However, the broader goal of this patch is to separate the information-gathering part of the analysis to the diagnostic-flagging part of the analysis, with the hope that we can maybe derive the former automatically while still allowing the latter to be written manually. It seems to me that it would be infeasible to automatically derive both.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127898/new/

https://reviews.llvm.org/D127898