[clang-tools-extra] r354517 - [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer
via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 20 16:57:22 PST 2019
Hi Jonas,
Your commit seems to hit a compilation error on our internal build bot when built on Windows using Visual Studio 2015. Can you take a look?
ExceptionAnalyzer.cpp
c:\src\upstream\llvm_clean\tools\clang\tools\extra\clang-tidy\utils\ExceptionAnalyzer.h(112): error C3431: 'State': a scoped enumeration cannot be redeclared as an unscoped enumeration [C:\src\upstream\354517-win32-Release\tools\clang\tools\extra\clang-tidy\utils\clangTidyUtils.vcxproj]
Douglas Yung
-----Original Message-----
From: cfe-commits <cfe-commits-bounces at lists.llvm.org> On Behalf Of Jonas Toth via cfe-commits
Sent: Wednesday, February 20, 2019 13:05
To: cfe-commits at lists.llvm.org
Subject: [clang-tools-extra] r354517 - [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer
Author: jonastoth
Date: Wed Feb 20 13:04:36 2019
New Revision: 354517
URL: http://llvm.org/viewvc/llvm-project?rev=354517&view=rev
Log:
[clang-tidy] refactor ExceptionAnalyzer further to give ternary answer
Summary:
The analsis on the throwing behvaiour on functions and statements gave only a binary answer whether an exception could occur and if yes which types are thrown.
This refactoring allows keeping track if there is a unknown factor, because the code calls to some functions with unavailable source code with no `noexcept` information.
This 'potential Unknown' information is propagated properly and can be queried separately.
Reviewers: lebedev.ri, aaron.ballman, baloghadamsoftware, alexfh
Reviewed By: lebedev.ri, baloghadamsoftware
Subscribers: xazax.hun, rnkovacs, a.sidorin, Szelethus, donat.nagy, dkrupp, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D57883
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.cpp
clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.h
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp?rev=354517&r1=354516&r2=354517&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ Wed Feb 20 13:04:36 2019
@@ -42,6 +42,7 @@ ExceptionEscapeCheck::ExceptionEscapeChe
IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),
IgnoredExceptionsVec.end());
Tracer.ignoreExceptions(std::move(IgnoredExceptions));
+ Tracer.ignoreBadAlloc(true);
}
void ExceptionEscapeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -70,7 +71,8 @@ void ExceptionEscapeCheck::check(const M
if (!MatchedDecl)
return;
- if (Tracer.throwsException(MatchedDecl))
+ if (Tracer.analyze(MatchedDecl).getBehaviour() ==
+ utils::ExceptionAnalyzer::State::Throwing)
// FIXME: We should provide more information about the exact location where
// the exception is thrown, maybe the full path the exception escapes
diag(MatchedDecl->getLocation(),
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.cpp?rev=354517&r1=354516&r2=354517&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.cpp Wed
+++ Feb 20 13:04:36 2019
@@ -8,10 +8,44 @@
#include "ExceptionAnalyzer.h"
-#include "clang/AST/ASTContext.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
-
namespace clang {
+namespace tidy {
+namespace utils {
+
+void ExceptionAnalyzer::ExceptionInfo::registerException(
+ const Type *ExceptionType) {
+ assert(ExceptionType != nullptr && "Only valid types are accepted");
+ Behaviour = State::Throwing;
+ ThrownExceptions.insert(ExceptionType);
+}
+
+void ExceptionAnalyzer::ExceptionInfo::registerExceptions(
+ const Throwables &Exceptions) {
+ if (Exceptions.size() == 0)
+ return;
+ Behaviour = State::Throwing;
+ ThrownExceptions.insert(Exceptions.begin(), Exceptions.end()); }
+
+ExceptionAnalyzer::ExceptionInfo &ExceptionAnalyzer::ExceptionInfo::merge(
+ const ExceptionAnalyzer::ExceptionInfo &Other) {
+ // Only the following two cases require an update to the local
+ // 'Behaviour'. If the local entity is already throwing there will be
+no
+ // change and if the other entity is throwing the merged entity will
+throw
+ // as well.
+ // If one of both entities is 'Unknown' and the other one does not
+throw
+ // the merged entity is 'Unknown' as well.
+ if (Other.Behaviour == State::Throwing)
+ Behaviour = State::Throwing;
+ else if (Other.Behaviour == State::Unknown && Behaviour == State::NotThrowing)
+ Behaviour = State::Unknown;
+
+ ContainsUnknown = ContainsUnknown || Other.ContainsUnknown;
+ ThrownExceptions.insert(Other.ThrownExceptions.begin(),
+ Other.ThrownExceptions.end());
+ return *this;
+}
+
static bool isBaseOf(const Type *DerivedType, const Type *BaseType) {
const auto *DerivedClass = DerivedType->getAsCXXRecordDecl();
const auto *BaseClass = BaseType->getAsCXXRecordDecl(); @@ -22,36 +56,87 @@ static bool isBaseOf(const Type *Derived
[BaseClass](const CXXRecordDecl *Cur) { return Cur != BaseClass; }); }
-namespace tidy {
-namespace utils {
+bool ExceptionAnalyzer::ExceptionInfo::filterByCatch(const Type
+*BaseClass) {
+ llvm::SmallVector<const Type *, 8> TypesToDelete;
+ for (const Type *T : ThrownExceptions) {
+ if (T == BaseClass || isBaseOf(T, BaseClass))
+ TypesToDelete.push_back(T);
+ }
+
+ for (const Type *T : TypesToDelete)
+ ThrownExceptions.erase(T);
+
+ reevaluateBehaviour();
+ return TypesToDelete.size() > 0;
+}
+
+ExceptionAnalyzer::ExceptionInfo &
+ExceptionAnalyzer::ExceptionInfo::filterIgnoredExceptions(
+ const llvm::StringSet<> &IgnoredTypes, bool IgnoreBadAlloc) {
+ llvm::SmallVector<const Type *, 8> TypesToDelete;
+ // Note: Using a 'SmallSet' with 'llvm::remove_if()' is not possible.
+ // Therefore this slightly hacky implementation is required.
+ for (const Type *T : ThrownExceptions) {
+ if (const auto *TD = T->getAsTagDecl()) {
+ if (TD->getDeclName().isIdentifier()) {
+ if ((IgnoreBadAlloc &&
+ (TD->getName() == "bad_alloc" && TD->isInStdNamespace())) ||
+ (IgnoredTypes.count(TD->getName()) > 0))
+ TypesToDelete.push_back(T);
+ }
+ }
+ }
+ for (const Type *T : TypesToDelete)
+ ThrownExceptions.erase(T);
+
+ reevaluateBehaviour();
+ return *this;
+}
+
+void ExceptionAnalyzer::ExceptionInfo::clear() {
+ Behaviour = State::NotThrowing;
+ ContainsUnknown = false;
+ ThrownExceptions.clear();
+}
-ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
+void ExceptionAnalyzer::ExceptionInfo::reevaluateBehaviour() {
+ if (ThrownExceptions.size() == 0)
+ if (ContainsUnknown)
+ Behaviour = State::Unknown;
+ else
+ Behaviour = State::NotThrowing;
+ else
+ Behaviour = State::Throwing;
+}
+
+ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
const FunctionDecl *Func,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
if (CallStack.count(Func))
- return TypeVec();
+ return ExceptionInfo::createNonThrowing();
if (const Stmt *Body = Func->getBody()) {
CallStack.insert(Func);
- const TypeVec Result = throwsException(Body, TypeVec(), CallStack);
+ ExceptionInfo Result =
+ throwsException(Body, ExceptionInfo::Throwables(), CallStack);
CallStack.erase(Func);
return Result;
}
- TypeVec Result;
+ auto Result = ExceptionInfo::createUnknown();
if (const auto *FPT = Func->getType()->getAs<FunctionProtoType>()) {
- for (const QualType Ex : FPT->exceptions()) {
- Result.push_back(Ex.getTypePtr());
- }
+ for (const QualType Ex : FPT->exceptions())
+ Result.registerException(Ex.getTypePtr());
}
return Result;
}
-ExceptionAnalyzer::TypeVec ExceptionAnalyzer::throwsException(
- const Stmt *St, const TypeVec &Caught,
+/// Analyzes a single statment on it's throwing behaviour. This is in
+principle /// possible except some 'Unknown' functions are called.
+ExceptionAnalyzer::ExceptionInfo ExceptionAnalyzer::throwsException(
+ const Stmt *St, const ExceptionInfo::Throwables &Caught,
llvm::SmallSet<const FunctionDecl *, 32> &CallStack) {
- TypeVec Results;
-
+ auto Results = ExceptionInfo::createNonThrowing();
if (!St)
return Results;
@@ -59,28 +144,28 @@ ExceptionAnalyzer::TypeVec ExceptionAnal
if (const auto *ThrownExpr = Throw->getSubExpr()) {
const auto *ThrownType =
ThrownExpr->getType()->getUnqualifiedDesugaredType();
- if (ThrownType->isReferenceType()) {
+ if (ThrownType->isReferenceType())
ThrownType = ThrownType->castAs<ReferenceType>()
->getPointeeType()
->getUnqualifiedDesugaredType();
- }
- if (const auto *TD = ThrownType->getAsTagDecl()) {
- if (TD->getDeclName().isIdentifier() && TD->getName() == "bad_alloc" &&
- TD->isInStdNamespace())
- return Results;
- }
- Results.push_back(ThrownExpr->getType()->getUnqualifiedDesugaredType());
- } else {
- Results.append(Caught.begin(), Caught.end());
- }
+ Results.registerException(
+ ThrownExpr->getType()->getUnqualifiedDesugaredType());
+ } else
+ // A rethrow of a caught exception happens which makes it possible
+ // to throw all exception that are caught in the 'catch' clause of
+ // the parent try-catch block.
+ Results.registerExceptions(Caught);
} else if (const auto *Try = dyn_cast<CXXTryStmt>(St)) {
- TypeVec Uncaught = throwsException(Try->getTryBlock(), Caught, CallStack);
+ ExceptionInfo Uncaught =
+ throwsException(Try->getTryBlock(), Caught, CallStack);
for (unsigned i = 0; i < Try->getNumHandlers(); ++i) {
const CXXCatchStmt *Catch = Try->getHandler(i);
+
+ // Everything is catched through 'catch(...)'.
if (!Catch->getExceptionDecl()) {
- const TypeVec Rethrown =
- throwsException(Catch->getHandlerBlock(), Uncaught, CallStack);
- Results.append(Rethrown.begin(), Rethrown.end());
+ ExceptionInfo Rethrown = throwsException(
+ Catch->getHandlerBlock(), Uncaught.getExceptionTypes(), CallStack);
+ Results.merge(Rethrown);
Uncaught.clear();
} else {
const auto *CaughtType =
@@ -90,64 +175,62 @@ ExceptionAnalyzer::TypeVec ExceptionAnal
->getPointeeType()
->getUnqualifiedDesugaredType();
}
- auto NewEnd =
- llvm::remove_if(Uncaught, [&CaughtType](const Type *ThrownType) {
- return ThrownType == CaughtType ||
- isBaseOf(ThrownType, CaughtType);
- });
- if (NewEnd != Uncaught.end()) {
- Uncaught.erase(NewEnd, Uncaught.end());
- const TypeVec Rethrown = throwsException(
- Catch->getHandlerBlock(), TypeVec(1, CaughtType), CallStack);
- Results.append(Rethrown.begin(), Rethrown.end());
+
+ // If the caught exception will catch multiple previously potential
+ // thrown types (because it's sensitive to inheritance) the throwing
+ // situation changes. First of all filter the exception types and
+ // analyze if the baseclass-exception is rethrown.
+ if (Uncaught.filterByCatch(CaughtType)) {
+ ExceptionInfo::Throwables CaughtExceptions;
+ CaughtExceptions.insert(CaughtType);
+ ExceptionInfo Rethrown = throwsException(Catch->getHandlerBlock(),
+ CaughtExceptions, CallStack);
+ Results.merge(Rethrown);
}
}
}
- Results.append(Uncaught.begin(), Uncaught.end());
+ Results.merge(Uncaught);
} else if (const auto *Call = dyn_cast<CallExpr>(St)) {
if (const FunctionDecl *Func = Call->getDirectCallee()) {
- TypeVec Excs = throwsException(Func, CallStack);
- Results.append(Excs.begin(), Excs.end());
+ ExceptionInfo Excs = throwsException(Func, CallStack);
+ Results.merge(Excs);
}
} else {
for (const Stmt *Child : St->children()) {
- TypeVec Excs = throwsException(Child, Caught, CallStack);
- Results.append(Excs.begin(), Excs.end());
+ ExceptionInfo Excs = throwsException(Child, Caught, CallStack);
+ Results.merge(Excs);
}
}
return Results;
}
-bool ExceptionAnalyzer::throwsException(const FunctionDecl *Func) {
- // Check if the function has already been analyzed and reuse that result.
- if (FunctionCache.count(Func) > 0)
- return FunctionCache[Func];
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+ ExceptionInfo ExceptionList;
- llvm::SmallSet<const FunctionDecl *, 32> CallStack;
- TypeVec ExceptionList = throwsException(Func, CallStack);
+ // Check if the function has already been analyzed and reuse that result.
+ if (FunctionCache.count(Func) == 0) {
+ llvm::SmallSet<const FunctionDecl *, 32> CallStack;
+ ExceptionList = throwsException(Func, CallStack);
+
+ // Cache the result of the analysis. This is done prior to filtering
+ // because it is best to keep as much information as possible.
+ // The results here might be relevant to different analysis passes
+ // with different needs as well.
+ FunctionCache.insert(std::make_pair(Func, ExceptionList)); } else
+ ExceptionList = FunctionCache[Func];
+
+ if (ExceptionList.getBehaviour() == State::NotThrowing ||
+ ExceptionList.getBehaviour() == State::Unknown)
+ return ExceptionList;
// Remove all ignored exceptions from the list of exceptions that can be
// thrown.
- auto NewEnd = llvm::remove_if(ExceptionList, [this](const Type *Exception) {
- return isIgnoredExceptionType(Exception);
- });
- ExceptionList.erase(NewEnd, ExceptionList.end());
-
- // Cache the result of the analysis.
- bool FunctionThrows = ExceptionList.size() > 0;
- FunctionCache.insert(std::make_pair(Func, FunctionThrows));
-
- return FunctionThrows;
-}
-
-bool ExceptionAnalyzer::isIgnoredExceptionType(const Type *Exception) {
- if (const auto *TD = Exception->getAsTagDecl()) {
- if (TD->getDeclName().isIdentifier())
- return IgnoredExceptions.count(TD->getName()) > 0;
- }
- return false;
-}
+ ExceptionList.filterIgnoredExceptions(IgnoredExceptions,
+ IgnoreBadAlloc);
+ return ExceptionList;
+}
} // namespace utils
} // namespace tidy
Modified: clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.h?rev=354517&r1=354516&r2=354517&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.h (original)
+++ clang-tools-extra/trunk/clang-tidy/utils/ExceptionAnalyzer.h Wed Feb
+++ 20 13:04:36 2019
@@ -10,6 +10,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_EXCEPTION_ANALYZER_H
#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/StringSet.h"
@@ -17,29 +18,131 @@ namespace clang {
namespace tidy {
namespace utils {
-/// This class analysis if a `FunctionDecl` can in principle throw an exception, -/// either directly or indirectly.
-/// It can be configured to ignore custom exception types.
+/// This class analysis if a `FunctionDecl` can in principle throw an
+/// exception, either directly or indirectly. It can be configured to
+ignore /// custom exception types.
class ExceptionAnalyzer {
public:
+ enum class State : std::int8_t {
+ Throwing = 0, ///< The function can definitly throw given an AST.
+ NotThrowing = 1, ///< This function can not throw, given an AST.
+ Unknown = 2, ///< This can happen for extern functions without available
+ ///< definition.
+ };
+
+ /// Bundle the gathered information about an entity like a function
+ regarding /// it's exception behaviour. The 'NonThrowing'-state can
+ be considered as the /// neutral element in terms of information propagation.
+ /// In the case of 'Throwing' state it is possible that 'getExceptionTypes'
+ /// does not include *ALL* possible types as there is the possibility
+ that /// an 'Unknown' function is called that might throw a
+ previously unknown /// exception at runtime.
+ class ExceptionInfo {
+ public:
+ using Throwables = llvm::SmallSet<const Type *, 2>;
+ static ExceptionInfo createUnknown() {
+ return ExceptionInfo(State::Unknown);
+ }
+ static ExceptionInfo createNonThrowing() {
+ return ExceptionInfo(State::Throwing);
+ }
+
+ /// By default the exception situation is unknown and must be
+ /// clarified step-wise.
+ ExceptionInfo() : Behaviour(State::NotThrowing), ContainsUnknown(false) {}
+ ExceptionInfo(State S)
+ : Behaviour(S), ContainsUnknown(S == State::Unknown) {}
+
+ ExceptionInfo(const ExceptionInfo &) = default;
+ ExceptionInfo &operator=(const ExceptionInfo &) = default;
+ ExceptionInfo(ExceptionInfo &&) = default;
+ ExceptionInfo &operator=(ExceptionInfo &&) = default;
+
+ State getBehaviour() const { return Behaviour; }
+
+ /// Register a single exception type as recognized potential exception to be
+ /// thrown.
+ void registerException(const Type *ExceptionType);
+
+ /// Registers a `SmallVector` of exception types as recognized potential
+ /// exceptions to be thrown.
+ void registerExceptions(const Throwables &Exceptions);
+
+ /// Updates the local state according to the other state. That means if
+ /// for example a function contains multiple statements the 'ExceptionInfo'
+ /// for the final function is the merged result of each statement.
+ /// If one of these statements throws the whole function throws and if one
+ /// part is unknown and the rest is non-throwing the result will be
+ /// unknown.
+ ExceptionInfo &merge(const ExceptionInfo &Other);
+
+ /// This method is useful in case 'catch' clauses are analyzed as it is
+ /// possible to catch multiple exception types by one 'catch' if they
+ /// are a subclass of the 'catch'ed exception type.
+ /// Returns 'true' if some exceptions were filtered, otherwise 'false'.
+ bool filterByCatch(const Type *BaseClass);
+
+ /// Filter the set of thrown exception type against a set of ignored
+ /// types that shall not be considered in the exception analysis.
+ /// This includes explicit `std::bad_alloc` ignoring as separate option.
+ ExceptionInfo &
+ filterIgnoredExceptions(const llvm::StringSet<> &IgnoredTypes,
+ bool IgnoreBadAlloc);
+
+ /// Clear the state to 'NonThrowing' to make the corresponding entity
+ /// neutral.
+ void clear();
+
+ /// References the set of known exception types that can escape from the
+ /// corresponding entity.
+ const Throwables &getExceptionTypes() const { return
+ ThrownExceptions; }
+
+ /// Signal if the there is any 'Unknown' element within the scope of
+ /// the related entity. This might be relevant if the entity is 'Throwing'
+ /// and to ensure that no other exception then 'getExceptionTypes' can
+ /// occur. If there is an 'Unknown' element this can not be guaranteed.
+ bool containsUnknownElements() const { return ContainsUnknown; }
+
+ private:
+ /// Recalculate the 'Behaviour' for example after filtering.
+ void reevaluateBehaviour();
+
+ /// Keep track if the entity related to this 'ExceptionInfo' can in princple
+ /// throw, if it's unknown or if it won't throw.
+ enum State Behaviour : 2;
+
+ /// Keep track if the entity contains any unknown elements to keep track
+ /// of the certainty of decisions and/or correct 'Behaviour' transition
+ /// after filtering.
+ bool ContainsUnknown : 1;
+
+ /// 'ThrownException' is empty if the 'Behaviour' is either 'NotThrowing' or
+ /// 'Unknown'.
+ Throwables ThrownExceptions;
+ };
+ static_assert(sizeof(ExceptionInfo) <= 64u,
+ "size of exceptioninfo shall be at max one cache
+ line");
+
ExceptionAnalyzer() = default;
- bool throwsException(const FunctionDecl *Func);
+ void ignoreBadAlloc(bool ShallIgnore) { IgnoreBadAlloc = ShallIgnore;
+ }
void ignoreExceptions(llvm::StringSet<> ExceptionNames) {
IgnoredExceptions = std::move(ExceptionNames);
}
-private:
- using TypeVec = llvm::SmallVector<const Type *, 8>;
+ ExceptionInfo analyze(const FunctionDecl *Func);
- TypeVec throwsException(const FunctionDecl *Func,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
- TypeVec throwsException(const Stmt *St, const TypeVec &Caught,
- llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
- bool isIgnoredExceptionType(const Type *Exception);
+private:
+ ExceptionInfo
+ throwsException(const FunctionDecl *Func,
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ ExceptionInfo
+ throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
+ llvm::SmallSet<const FunctionDecl *, 32> &CallStack);
+ bool IgnoreBadAlloc = true;
llvm::StringSet<> IgnoredExceptions;
- llvm::DenseMap<const FunctionDecl *, bool> FunctionCache;
+ std::map<const FunctionDecl *, ExceptionInfo> FunctionCache;
};
} // namespace utils
} // namespace tidy
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list