[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer
Jonas Toth via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 7 07:34:26 PST 2019
JonasToth marked 2 inline comments as done.
JonasToth added inline comments.
================
Comment at: clang-tidy/utils/ExceptionAnalyzer.h:31-192
+enum class ExceptionState : std::int8_t {
+ Throwing, ///< The function can definitly throw given an AST.
+ NotThrowing, ///< This function can not throw, given an AST.
+ Unknown, ///< This can happen for extern functions without available
+ ///< definition.
+};
+
----------------
lebedev.ri wrote:
> Would it be better to add them into `ExceptionAnalyzer` class?
>
`ThrownExceptions`? I think it fits here. Removing would shrink `ExceptionInfo` (and `ContainsUnknown` and `State` could be packed into one byte) which is a plus.
================
Comment at: clang-tidy/utils/ExceptionAnalyzer.h:218
llvm::StringSet<> IgnoredExceptions;
- llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+ std::map<const FunctionDecl *, ExceptionInfo> FunctionCache;
};
----------------
lebedev.ri wrote:
> Why can't `llvm::DenseMap` continue to be used?
I would need to add traits for `ExceptionInfo` to work with `DenseMap`. So i went this way :)
>From the docs `DenseMap` shall map small types (like ptr-ptr) but the `ExceptionInfo` has the set of types as `SmallSet<Type*, 8>`, so not sure if that is actually a good fit.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57883/new/
https://reviews.llvm.org/D57883
More information about the cfe-commits
mailing list