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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
