[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 22 06:28:22 PDT 2019


lebedev.ri added inline comments.


================
Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;
----------------
gribozavr wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > Please bikeshed on the name. I don't think this one is good.
> > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, `analyzeAbstract`, something good in these?
> > 
> > Given its private its not too important either ;)
> I'd suggest to simplify by changing `analyzeBoilerplate()` into a non-template, into this specifically:
> 
> ```
> ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) {
>     if (ExceptionList.getBehaviour() == State::NotThrowing ||
>       ExceptionList.getBehaviour() == State::Unknown)
>     return ExceptionList;
> 
>   // Remove all ignored exceptions from the list of exceptions that can be
>   // thrown.
>   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
> 
>   return ExceptionList;
> }
> ```
> 
> And then call it in `analyze()`:
> 
> ```
> ExceptionAnalyzer::ExceptionInfo
> ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
>   return filterIgnoredExceptions(analyzeImpl(Func));
> }
> ```
Hmm not really.
I intentionally did all this to maximally complicate any possibility of accidentally doing
something different given diferent entry point (`Stmt` vs `FunctionDecl`).
Refactoring it that way, via `filterIgnoredExceptions()` increases that risk back.
(accidentally omit that intermediate function, and ...)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650





More information about the cfe-commits mailing list