xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

Overall this looks good to me. However, I think this might not use the full 
potential of the check itself. With the information that the dataflow framework 
have it could distinguish between **potentially** unsafe accesses and 
**provably** unsafe accesses depending on whether the `has_value` property is 
constrained to be false. From the user point of view, it would be nice to emit 
different warning messages for the above two cases. This can help to gradually 
introduce this check to a larger codebase and focus on the higher severity 
diagnostics first.



================
Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst:29
+
+Checking if a value exists, then accessing the value
+----------------------------------------------------
----------------
I wonder if it would be easier to read if we had two top level categories, one 
for safe and one for unsafe accesses instead of switching back and forth 
between examples.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121120

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to