[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:39-45
+  using dataflow::ControlFlowContext;
+  using dataflow::DataflowAnalysisContext;
+  using dataflow::DataflowAnalysisState;
+  using dataflow::Environment;
+  using dataflow::UncheckedOptionalAccessModel;
+  using dataflow::WatchedLiteralsSolver;
+  using llvm::Expected;
----------------
ymandel wrote:
> sgatev wrote:
> > Do we really need all these using declarations? There seems to be one reference for each of these types. I think we can simply qualify the references.
> No, we don't need them, but IMO they clarify the code. We're really heavy with the types, given that `auto` is discouraged, so I think pulling these out improves readability in some critical places (lines 56 and 63). I figured I'd do all for consistency. I've removed those not related to lines 56 and 63, or used more than once. WDYT?
Ah, I see, we're using all of these types in a single line :D Perhaps `std::vector<Optional<DataflowAnalysisState<T>>>` could be encapsulated in a `DataflowResult<T>` type or something like that. I suggest we keep the patch as it currently is and consider such a change separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121120